suspend: rockchip: add suspend and resume of rk3288 [chromiumos/third_party/kernel : chromeos-3.14]

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to do a code review. Please visit

https://chromium-review.googlesource.com/215780

to review the following change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=None
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
5 files changed, 732 insertions(+), 0 deletions(-)



diff --git a/arch/arm/mach-rockchip/Makefile
b/arch/arm/mach-rockchip/Makefile
index 4377a14..2b15094 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
+obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
diff --git a/arch/arm/mach-rockchip/pm.c b/arch/arm/mach-rockchip/pm.c
new file mode 100644
index 0000000..9b703a6
--- /dev/null
+++ b/arch/arm/mach-rockchip/pm.c
@@ -0,0 +1,478 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony...@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/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/suspend.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <asm/cacheflush.h>
+#include <asm/tlbflush.h>
+#include <asm/suspend.h>
+#include "pm.h"
+
+static char bootram_save_data[RK3288_BOOTRAM_SIZE];
+static u32 bootdata_data[RKPM_BOOTDATA_ARR_SIZE];
+static void __iomem *rk3288_pmu_base;
+static void __iomem *rk3288_cru_base;
+static void __iomem *rk3288_bootram_base;
+static void __iomem *rk3288_sgrf_base;
+
+static struct rk_cpu_resume_data rk_cpu_data;
+
+void rk_cpu_resume_data_init(void)
+{
+ rk_cpu_data.bootram_save = (char *)bootram_save_data;
+ rk_cpu_data.bootram_save_size = sizeof(bootram_save_data);
+
+ rk_cpu_data.bootram_base = (char *)rk3288_bootram_base;
+ rk_cpu_data.bootram_phy = (char *)RK3288_BOOTRAM_PHYS;
+ rk_cpu_data.bootram_size = RK3288_BOOTRAM_SIZE;
+
+ rk_cpu_data.bootcode_base = rk_cpu_data.bootram_base
+ +RKPM_BOOT_CODE_OFFSET;
+
+ rk_cpu_data.bootcode_phy = rk_cpu_data.bootram_phy
+ +RKPM_BOOT_CODE_OFFSET;
+
+ rk_cpu_data.bootcode_src = (char *)rockchip_slp_cpu_resume;
+ rk_cpu_data.bootcode_size = RKPM_BOOT_CODE_SIZE;
+
+ rk_cpu_data.bootdata_base = rk_cpu_data.bootram_base
+ +RKPM_BOOT_DATA_OFFSET;
+
+ rk_cpu_data.bootdata_phy = rk_cpu_data.bootram_phy
+ +RKPM_BOOT_DATA_OFFSET;
+
+ rk_cpu_data.bootdata_src = (char *)bootdata_data;
+ rk_cpu_data.bootdata_size = sizeof(bootdata_data);
+}
+
+static void sram_data_for_sleep(struct rk_cpu_resume_data *rdata)
+{
+ unsigned long data_base, data_end;
+
+ /* save root sram data in ddr mem */
+ memcpy(rdata->bootram_save, rdata->bootram_base,
+ rdata->bootram_size);
+
+ /* move resume code and date to bootsram */
+ memcpy(rdata->bootcode_base, rdata->bootcode_src,
+ rdata->bootcode_size);
+
+ memcpy(rdata->bootdata_base, rdata->bootdata_src,
+ rdata->bootdata_size);
+
+ data_base = (unsigned long)rdata->bootram_base;
+ data_end = (unsigned long)rdata->bootram_base + rdata->bootram_size;
+ flush_icache_range(data_base, data_end);
+
+ data_base = (unsigned long)rdata->bootram_phy;
+ data_end = (unsigned long)rdata->bootram_phy + rdata->bootram_size;
+ outer_clean_range(data_base, data_end);
+}
+
+static void sram_data_resume(struct rk_cpu_resume_data *rdata)
+{
+ unsigned long data_base, data_end;
+
+ memcpy(rdata->bootram_save, rdata->bootram_base, rdata->bootram_size);
+
+ data_base = (unsigned long)rdata->bootram_base;
+ data_end = (unsigned long)rdata->bootram_base + rdata->bootram_size;
+ flush_icache_range(data_base, data_end);
+
+ data_base = (unsigned long)rdata->bootram_phy;
+ data_end = (unsigned long)rdata->bootram_phy + rdata->bootram_size;
+ outer_clean_range(data_base, data_end);
+}
+
+static inline u32 rk3288_l2_config(void)
+{
+ u32 l2ctlr;
+
+ asm("mrc p15, 1, %0, c9, c0, 2" : "=r" (l2ctlr));
+ return l2ctlr;
+}
+
+static inline u32 rk3288_armerrata_818325(void)
+{
+ u32 armerrata;
+
+ asm("mrc p15, 0, %0, c15, c0, 1" : "=r" (armerrata));
+ return armerrata;
+}
+
+static void rk3288_pmupin_setfun(u8 port, u8 bank, u8 b_gpio, u8 fun)
+{
+ u32 off_set, val;
+
+ bank -= 0xa;
+
+ if (port == 0) {
+ if (bank > 2)
+ return;
+
+ off_set = RK3288_PMU_GPIO0_A_IOMUX+bank*4;
+
+ val = readl_relaxed(rk3288_pmu_base+off_set);
+ val = RKPM_VAL_SETBITS(val, fun, b_gpio*2, 0x3);
+
+ writel_relaxed(val, rk3288_pmu_base+off_set);
+ }
+}
+
+static void rk3288_sram_data_prepare(u32 pwrmode)
+{
+ if (pwrmode&(BIT(pmu_scu_en)|BIT(pmu_a12_0_pd_en))) {
+ bootdata_data[RKPM_BOOTDATA_L2LTY_F] = 1;
+ bootdata_data[RKPM_BOOTDATA_L2LTY] = rk3288_l2_config();
+ bootdata_data[RKPM_BOOTDATA_ARM_ERRATA_818325_F] = 1;
+
+ bootdata_data[RKPM_BOOTDATA_ARM_ERRATA_818325] =
+ rk3288_armerrata_818325();
+
+ bootdata_data[RKPM_BOOTDATA_CPUSP] = RK3288_BOOTRAM_PHYS +
+ ((RK3288_BOOTRAM_SIZE-1)&~0x7);
+
+ bootdata_data[RKPM_BOOTDATA_CPUCODE] = virt_to_phys(cpu_resume);
+ } else {
+ bootdata_data[RKPM_BOOTDATA_L2LTY_F] = 0;
+ bootdata_data[RKPM_BOOTDATA_ARM_ERRATA_818325_F] = 0;
+ bootdata_data[RKPM_BOOTDATA_CPUCODE] = 0;
+ bootdata_data[RKPM_BOOTDATA_DDR_F] = 0;
+ }
+
+ sram_data_for_sleep(&rk_cpu_data);
+ flush_cache_all();
+ outer_flush_all();
+ local_flush_tlb_all();
+}
+
+static u32 rk3288_powermode;
+static u32 rk3288_sgrf_soc_con0, rk3288_pmu_wakeup_cfg0,
+ rk3288_pmu_wakeup_cfg1, rk3288_pmu_pwr_mode_con0,
+ rk3288_pmu_pwr_mode_con1;
+
+static u32 rk3288_slp_mode_set(int level)
+{
+ u32 mode_set, mode_set1;
+
+ rk3288_sgrf_soc_con0 =
+ readl_relaxed(rk3288_sgrf_base+RK3288_SGRF_SOC_CON0);
+
+ rk3288_pmu_wakeup_cfg0 =
+ readl_relaxed(rk3288_pmu_base+RK3288_PMU_WAKEUP_CFG0);
+
+ rk3288_pmu_wakeup_cfg1 =
+ readl_relaxed(rk3288_pmu_base+RK3288_PMU_WAKEUP_CFG1);
+
+ rk3288_pmu_pwr_mode_con0 =
+ readl_relaxed(rk3288_pmu_base+RK3288_PMU_PWRMODE_CON);
+
+ rk3288_pmu_pwr_mode_con1 =
+ readl_relaxed(rk3288_pmu_base+RK3288_PMU_PWRMODE_CON1);
+
+ /* setting gpio0_a0 arm off pin */
+ rk3288_pmupin_setfun(0x0, 0xa, 0x0, 0x0);
+
+ /*
+ * 0x1<<0 armint wakup ,
+ * if unused clk is gated in supend,it is Recommend.
+ * 0x1<<3, only gpio can wake up sys.
+ */
+ writel_relaxed(0x1<<3, rk3288_pmu_base+RK3288_PMU_WAKEUP_CFG1);
+ dsb();
+
+ /* enable boot ram */
+ writel_relaxed((0x1<<8)|(0x1<<(8+16)),
+ rk3288_sgrf_base+RK3288_SGRF_SOC_CON0);
+ dsb();
+
+ writel_relaxed(RK3288_BOOTRAM_PHYS,
+ rk3288_sgrf_base+RK3288_SGRF_FAST_BOOT_ADDR);
+ dsb();
+
+ mode_set = BIT(pmu_chip_pd_en) | BIT(pmu_l2flush_en)
+ | BIT(pmu_sref0_enter_en) | BIT(pmu_sref1_enter_en)
+ | BIT(pmu_ddr0_gating_en) | BIT(pmu_ddr1_gating_en)
+ ;
+
+ mode_set1 = BIT(pmu_clr_core) | BIT(pmu_clr_cpup)
+ | BIT(pmu_clr_alive) | BIT(pmu_clr_peri)
+ | BIT(pmu_clr_bus) | BIT(pmu_clr_dma)
+ ;
+
+ if (level == 0) {
+ /*
+ * arm off,logic normal
+ * if pmu_clk_core_src_gate_en is not set,
+ * wakeup will be error
+ */
+ mode_set |= BIT(pmu_scu_en) | BIT(pmu_clk_core_src_gate_en);
+
+ } else if (level == 1) {
+ /* arm off,logic dp */
+ mode_set |= BIT(pmu_scu_en) | 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)
+ ;
+ } else {
+ mode_set = 0;
+ mode_set1 = 0;
+ }
+
+ writel_relaxed(mode_set, rk3288_pmu_base + RK3288_PMU_PWRMODE_CON);
+ dsb();
+
+ writel_relaxed(mode_set1, rk3288_pmu_base + RK3288_PMU_PWRMODE_CON1);
+ dsb();
+
+ return readl_relaxed(rk3288_pmu_base + RK3288_PMU_PWRMODE_CON);
+}
+
+static void rk3288_slp_mode_set_resume(void)
+{
+ writel_relaxed(rk3288_pmu_wakeup_cfg0,
+ rk3288_pmu_base+RK3288_PMU_WAKEUP_CFG0);
+
+ writel_relaxed(rk3288_pmu_wakeup_cfg1,
+ rk3288_pmu_base+RK3288_PMU_WAKEUP_CFG1);
+
+ writel_relaxed(rk3288_pmu_pwr_mode_con0,
+ rk3288_pmu_base+RK3288_PMU_PWRMODE_CON);
+
+ writel_relaxed(rk3288_pmu_pwr_mode_con1,
+ rk3288_pmu_base+RK3288_PMU_PWRMODE_CON1);
+
+ writel_relaxed(rk3288_sgrf_soc_con0|(0x1<<(8+16)),
+ rk3288_sgrf_base+RK3288_SGRF_SOC_CON0);
+}
+
+#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)
+
+static u32 rk3288_cru_mode, rk3288_cru_sel0, rk3288_cru_sel1,
+ rk3288_cru_sel10, rk3288_cru_sel33, rk3288_cru_sel37;
+
+void rk3288_cru_save(void)
+{
+ /*
+ * cru will be set in fastboot by ic,
+ * so the following is save related reg
+ */
+ rk3288_cru_mode = readl_relaxed(rk3288_cru_base + RK3288_CRU_MODE_CON);
+ rk3288_cru_sel0 = readl_relaxed(rk3288_cru_base + RK3288_CRU_SEL0_CON);
+ rk3288_cru_sel1 = readl_relaxed(rk3288_cru_base + RK3288_CRU_SEL1_CON);
+
+ rk3288_cru_sel10 =
+ readl_relaxed(rk3288_cru_base + RK3288_CRU_SEL10_CON);
+
+ rk3288_cru_sel33 =
+ readl_relaxed(rk3288_cru_base + RK3288_CRU_SEL33_CON);
+
+ rk3288_cru_sel37 =
+ readl_relaxed(rk3288_cru_base + RK3288_CRU_SEL37_CON);
+
+ /* set pll into slow mode */
+ writel_relaxed(0xff0f0010, rk3288_cru_base + RK3288_CRU_MODE_CON);
+}
+
+void rk3288_cru_resume(void)
+{
+ writel_relaxed(rk3288_cru_mode | 0xffff0000,
+ rk3288_cru_base + RK3288_CRU_MODE_CON);
+
+ writel_relaxed(rk3288_cru_sel0 | 0xffff0000,
+ rk3288_cru_base + RK3288_CRU_SEL0_CON);
+
+ writel_relaxed(rk3288_cru_sel1 | 0xffff0000,
+ rk3288_cru_base + RK3288_CRU_SEL1_CON);
+
+ writel_relaxed(rk3288_cru_sel10 | 0xffff0000,
+ rk3288_cru_base + RK3288_CRU_SEL10_CON);
+
+ writel_relaxed(rk3288_cru_sel33 | 0xffff0000,
+ rk3288_cru_base + RK3288_CRU_SEL33_CON);
+
+ writel_relaxed(rk3288_cru_sel37 | 0xffff0000,
+ rk3288_cru_base + RK3288_CRU_SEL37_CON);
+}
+
+
+static void rk3288_save_setting(void)
+{
+ rk3288_powermode = rk3288_slp_mode_set(0);
+
+ if (rk3288_powermode & BIT(pmu_pwr_mode_en))
+ rk3288_sram_data_prepare(rk3288_powermode);
+
+ rk3288_cru_save();
+}
+
+static void rk3288_save_setting_resume(void)
+{
+ sram_data_resume(&rk_cpu_data);
+}
+
+static void rkpm_resume_first(void)
+{
+ rk3288_cru_resume();
+ rk3288_slp_mode_set_resume();
+}
+
+static int rockchip_lpmode_enter(unsigned long arg)
+{
+ local_flush_tlb_all();
+ flush_cache_all();
+ outer_flush_all();
+ outer_disable();
+ flush_cache_all();
+
+ dsb();
+ wfi();
+ dsb();
+ return 0;
+}
+
+static int rk3288_suspend_enter(suspend_state_t state)
+{
+ pr_info("%s\n", __func__);
+
+ local_fiq_disable();
+
+ rk3288_save_setting();
+
+ cpu_suspend(0, rockchip_lpmode_enter);
+
+ rkpm_resume_first();
+
+ rk3288_save_setting_resume();
+
+ local_fiq_enable();
+
+ pr_info("%s-out\n", __func__);
+ return 0;
+}
+
+static int rk3288_suspend_iomap(void)
+{
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "rockchip,rk3288-pmu");
+ if (!node) {
+ pr_err("%s: could not find pmu dt node\n", __func__);
+ return -1;
+ }
+
+ rk3288_pmu_base = of_iomap(node, 0);
+ if (!rk3288_pmu_base) {
+ pr_err("%s: could not map pmu registers\n", __func__);
+ return -1;
+ }
+
+
+ node = of_find_compatible_node(NULL, NULL, "mmio-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 registers\n", __func__);
+ return -1;
+ }
+ node = of_find_compatible_node(NULL, NULL, "rockchip,rk3288-cru");
+ if (!node) {
+ pr_err("%s: could not find cru dt node\n", __func__);
+ return -1;
+ }
+
+ rk3288_cru_base = of_iomap(node, 0);
+ if (!rk3288_cru_base) {
+ pr_err("%s: could not map cru registers\n", __func__);
+ return -1;
+ }
+
+
+ rk3288_bootram_base = ioremap(RK3288_BOOTRAM_PHYS, RK3288_BOOTRAM_SIZE);
+ rk3288_sgrf_base = ioremap(RK3288_SGRF_PHYS, RK3288_SGRF_SIZE);
+
+ return 0;
+}
+
+static int rk3288_suspend_init(void)
+{
+ int ret;
+
+ ret = rk3288_suspend_iomap();
+ rk_cpu_resume_data_init();
+ return ret;
+}
+
+static const struct platform_suspend_ops rk3288_suspend_ops = {
+ .enter = rk3288_suspend_enter,
+ .valid = suspend_valid_only_mem,
+};
+
+static int rockchip_suspend_init_check
+ (struct rockchip_suspend_of_device_id *matches)
+{
+ int ret = -1;
+ struct of_device_id *id = &matches->id;
+
+ if (!id->compatible && !id->data)
+ return -1;
+ if (of_machine_is_compatible(id->compatible)) {
+ ret = matches->init();
+ if (!ret)
+ suspend_set_ops(id->data);
+ }
+ return ret;
+}
+
+static struct rockchip_suspend_of_device_id rockchip_suspend_dt_match[] = {
+ {
+ .id = {
+ .compatible = "rockchip,rk3288",
+ .data = (void *)&rk3288_suspend_ops,
+ },
+ .init = rk3288_suspend_init,
+ },
+ {},
+};
+
+int __init rockchip_suspend_init(void)
+{
+ int ret = -1;
+ struct rockchip_suspend_of_device_id *matches =
+ rockchip_suspend_dt_match;
+
+ while (&matches->id && matches->init) {
+ if (rockchip_suspend_init_check(matches))
+ break;
+ matches++;
+ }
+
+ return ret;
+}
+
diff --git a/arch/arm/mach-rockchip/pm.h b/arch/arm/mach-rockchip/pm.h
new file mode 100644
index 0000000..7f9540c
--- /dev/null
+++ b/arch/arm/mach-rockchip/pm.h
@@ -0,0 +1,180 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony...@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
+
+/* cpu sp addr */
+#define RKPM_BOOTDATA_CPUSP (0)
+/* cpu resume fun,phy base */
+#define RKPM_BOOTDATA_CPUCODE (RKPM_BOOTDATA_CPUSP+1)
+/* ddr flag , 1 need to resume ddr ctr */
+#define RKPM_BOOTDATA_DDR_F (RKPM_BOOTDATA_CPUCODE+1)
+/* ddr pll flag, 1 need to resume dpll */
+#define RKPM_BOOTDATA_DPLL_F (RKPM_BOOTDATA_DDR_F+1)
+/* ddr resume code ,phy base */
+#define RKPM_BOOTDATA_DDRCODE (RKPM_BOOTDATA_DPLL_F+1)
+/* ddr resume data ,phy base */
+#define RKPM_BOOTDATA_DDRDATA (RKPM_BOOTDATA_DDRCODE+1)
+/* for l2 resume */
+#define RKPM_BOOTDATA_L2LTY_F (RKPM_BOOTDATA_DDRDATA+1)
+#define RKPM_BOOTDATA_L2LTY (RKPM_BOOTDATA_L2LTY_F+1)
+/* for arm errata 81835 resume */
+#define RKPM_BOOTDATA_ARM_ERRATA_818325_F (RKPM_BOOTDATA_L2LTY+1)
+#define RKPM_BOOTDATA_ARM_ERRATA_818325
(RKPM_BOOTDATA_ARM_ERRATA_818325_F+1)
+#define RKPM_BOOTDATA_ARR_SIZE (RKPM_BOOTDATA_ARM_ERRATA_818325+1)
+
+
+#define RKPM_BOOT_CODE_OFFSET (0x0)
+#define RKPM_BOOT_CODE_SIZE (0x100)
+#define RKPM_BOOT_DATA_OFFSET (RKPM_BOOT_CODE_OFFSET+RKPM_BOOT_CODE_SIZE)
+#define RKPM_BOOT_DATA_SIZE (RKPM_BOOTDATA_ARR_SIZE*4)
+#define RKPM_BOOT_DDRCODE_OFFSET
(RKPM_BOOT_DATA_OFFSET+RKPM_BOOT_DATA_SIZE)
+
+#ifndef __ASSEMBLY__
+
+struct rk_cpu_resume_data {
+ /* boot ram info*/
+ char *bootram_base;
+ char *bootram_phy;
+ u32 bootram_size;
+ /* boot ram info save*/
+ char *bootram_save;
+ u32 bootram_save_size;
+ /* code info in boot ram*/
+ char *bootcode_base;
+ char *bootcode_phy;
+ char *bootcode_src;
+ u32 bootcode_size;
+ /* code data info in boot ram*/
+ char *bootdata_base;
+ char *bootdata_phy;
+ char *bootdata_src;
+ u32 bootdata_size;
+ /* intsram info */
+ char *intsram_base;
+ char *intsram_phy;
+ u32 intsram_size;
+ char *intram_save;
+};
+
+struct rockchip_suspend_of_device_id {
+ struct of_device_id id;
+
+ int (*init)(void);
+};
+
+void rockchip_slp_cpu_resume(void);
+int __init rockchip_suspend_init(void);
+
+#define RKPM_BITS_CLR_VAL(val, bits_shift, msk) (val&~(msk<<bits_shift))
+#define RKPM_SETBITS(bits, bits_shift, msk) (((bits)&(msk)) <<
(bits_shift))
+#define RKPM_VAL_SETBITS(val, bits, bits_shift, msk) \
+ (RKPM_BITS_CLR_VAL(val, bits_shift, msk) |\
+ RKPM_SETBITS(bits, bits_shift, msk))
+
+/******fllowing is rk3288 defined**********/
+#define RK3288_PMU_WAKEUP_CFG0 0x00
+#define RK3288_PMU_WAKEUP_CFG1 0x04
+#define RK3288_PMU_PWRDN_CON 0x08
+#define RK3288_PMU_PWRDN_ST 0x0c
+#define RK3288_PMU_IDLE_REQ 0x10
+#define RK3288_PMU_IDLE_ST 0x14
+#define RK3288_PMU_PWRMODE_CON 0x18
+#define RK3288_PMU_PWR_STATE 0x1c
+#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_SFT_CON 0x48
+#define RK3288_PMU_DDR_SREF_ST 0x4c
+#define RK3288_PMU_INT_CON 0x50
+#define RK3288_PMU_INT_ST 0x54
+#define RK3288_PMU_BOOT_ADDR_SEL 0x58
+#define RK3288_PMU_GRF_CON 0x5c
+#define RK3288_PMU_GPIO_SR 0x60
+#define RK3288_PMU_GPIO0_A_PULL 0x64
+#define RK3288_PMU_GPIO0_B_PULL 0x68
+#define RK3288_PMU_GPIO0_C_PULL 0x6c
+#define RK3288_PMU_GPIO0_A_DRV 0x70
+#define RK3288_PMU_GPIO0_B_DRV 0x74
+#define RK3288_PMU_GPIO0_C_DRV 0x78
+#define RK3288_PMU_GPIO_OP 0x7c
+#define RK3288_PMU_GPIO0_SEL18 0x80
+#define RK3288_PMU_GPIO0_A_IOMUX 0x84
+#define RK3288_PMU_GPIO0_B_IOMUX 0x88
+#define RK3288_PMU_GPIO0_C_IOMUX 0x8c
+#define RK3288_PMU_PWRMODE_CON1 0x90
+#define RK3288_PMU_SYS_REG0 0x94
+#define RK3288_PMU_SYS_REG1 0x98
+#define RK3288_PMU_SYS_REG2 0x9c
+#define RK3288_PMU_SYS_REG3 0xa0
+
+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,
+};
+
+#define RK3288_BOOTRAM_PHYS (0xFF720000)
+#define RK3288_BOOTRAM_SIZE SZ_4K
+
+#define RK3288_SGRF_PHYS (0xff740000)
+#define RK3288_SGRF_SIZE SZ_4K
+
+#define RK3288_SGRF_SOC_CON0 (0x0000)
+#define RK3288_SGRF_FAST_BOOT_ADDR (0x0120)
+
+#endif
+#endif
diff --git a/arch/arm/mach-rockchip/rockchip.c
b/arch/arm/mach-rockchip/rockchip.c
index dc33924..c1bc5c4 100644
--- a/arch/arm/mach-rockchip/rockchip.c
+++ b/arch/arm/mach-rockchip/rockchip.c
@@ -23,11 +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)
{
l2x0_of_init(0, ~0UL);
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+ rockchip_suspend_init();
}

static const char * const rockchip_board_dt_compat[] = {
diff --git a/arch/arm/mach-rockchip/sleep.S b/arch/arm/mach-rockchip/sleep.S
new file mode 100644
index 0000000..12fdec6
--- /dev/null
+++ b/arch/arm/mach-rockchip/sleep.S
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony...@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>
+#include "pm.h"
+
+.data
+.align
+
+ENTRY(rockchip_slp_cpu_resume)
+9: mov r1, r1
+ 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
+ beq cpu0Run
+cpu1loop:
+ mov r3, #50
+ wfene
+ b cpu1loop
+cpu0Run:
+1: mov r1, r1
+ /* get boot ram base */
+ adr r1, 9b
+ /* resume data offset, from ram base */
+ ldr r5, 8f
+ /* resume data addr */
+ add r5, r5, r1
+
+ ldr r3 , [r5, #(RKPM_BOOTDATA_L2LTY_F*4)]
+ cmp r3, #1
+ bne arm_errata__818325
+ ldr r3 , [r5, #(RKPM_BOOTDATA_L2LTY*4)]
+ mcr p15, 1, r3, c9, c0, 2
+
+arm_errata__818325:
+ ldr r3 , [r5, #(RKPM_BOOTDATA_ARM_ERRATA_818325_F*4)]
+ cmp r3, #1
+ bne sp_set
+ ldr r3 , [r5, #(RKPM_BOOTDATA_ARM_ERRATA_818325*4)]
+ mcreq p15, 0, r3, c15, c0, 1
+
+sp_set:
+ ldr sp, [r5, #(RKPM_BOOTDATA_CPUSP*4)]
+ /*get SLP_DDR_NEED_RES ,if it is 1 ,ddr need to reusme*/
+ ldr r3, [r5, #(RKPM_BOOTDATA_DDR_F*4)]
+ cmp r3, #1
+ bne res
+ /* ddr resume code */
+ ldr r1, [r5, #(RKPM_BOOTDATA_DDRCODE*4)]
+ /* ddr resume data */
+ ldr r0, [r5, #(RKPM_BOOTDATA_DDRDATA*4)]
+ blx r1
+res:
+ ldr pc, [r5, #(RKPM_BOOTDATA_CPUCODE*4)]
+8: .long (RKPM_BOOT_CODE_OFFSET + RKPM_BOOT_CODE_SIZE)
+ENDPROC(rockchip_slp_cpu_resume)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(51 comments)

Still trying to read / understand all of this, but here are early
comments. I'll try to keep looking at it tomorrow.

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 46: +RKPM_BOOT_CODE_OFFSET;
nit: I think + goes on previous line.


Line 69: memcpy(rdata->bootram_save, rdata->bootram_base,
Does anyone else use this code? Do we really need to save / restore it, or
can we just say that the memory at 0xff720000 is for use by this code?


Line 157: }
I think that in both cases right now you're setting DDR_F to 0,
right? ...is the SDRAM code not ready yet? There should be a comment
_somewhere_ sayin so.


Line 170: static u32 rk3288_slp_mode_set(int level)
* Please add a comment for this function saying what it takes and what it
returns.
* Add an enum for level. See below.
* nit: only one space after u32


Line 184: readl_relaxed(rk3288_pmu_base+RK3288_PMU_PWRMODE_CON);
Why do you need to keep the old values? Should only affect the next
suspend, right? ...or does it somehow affect cpu hotplug?


Line 190: rk3288_pmupin_setfun(0x0, 0xa, 0x0, 0x0);
Wait, what are you doing here? Changing GPIO0_A0 to a GPIO? Shouldn't it
be "function 1" so that we can communicate to the PMIC that we're turning
off?

Also: I don't think this belongs here. Can't we just set the proper
function through a pinctrl "hog"? ...or is there some reason you need to
do this _right here_? If we can remove this we can remove the whole
rk3288_pmupin_setfun().


Line 194: * if unused clk is gated in supend,it is Recommend.
Can you try rewriting this? I can't understand.


Line 198: dsb();
You really need all these dsb() calls? Why?


Line 201: writel_relaxed((0x1<<8)|(0x1<<(8+16)),
Maybe say: set bit 8 so that system will resume to FAST_BOOT_ADDR


Line 212: ;
nit: I think semicolons go on the previous line in Linux.


Line 235: mode_set = 0;
what case is this?


Line 245: return readl_relaxed(rk3288_pmu_base + RK3288_PMU_PWRMODE_CON);
Return something that's more kernel-specific (an enum of some sort), not
just the values of PWRMODE_CON.


Line 262: writel_relaxed(rk3288_sgrf_soc_con0|(0x1<<(8+16)),
why the extra magic here?


Line 267: #define RK3288_CRU_SEL0_CON (0x60)
What is so critical about these specific set of CRU registers? Is it
possible to restore them slightly later by registering w/
register_syscore_ops() in the clock driver? That would be a lot cleaner.

If you really need these here, please tell me why. Also: please make an
array of registers to save/restore so we can just loop over them.


Line 279: * cru will be set in fastboot by ic,
In "fastboot by ic"? What does this mean?

nit: line up "*"s


Line 296: writel_relaxed(0xff0f0010, rk3288_cru_base +
RK3288_CRU_MODE_CON);
This mode doesn't make sense to me. The upper is the write mask, right?
That means that 0xff0f0010 is the same as 0xff0f0000. You're masking off
the nybble containing the "1" anyway.

I guess that means you're not changing the mode of DPLL. Why? Please
comment.


Line 301: writel_relaxed(rk3288_cru_mode | 0xffff0000,
Shouldn't mode be last? I assume this will set PLLs back to fast mode but
they'll be running at a random speed, right?


Line 323: 0
Add an enum for the possible values. So:

rk3288_slp_mode_set(RK3288_ARM_OFF_LOGIC_NORMAL);


Line 331: static void rk3288_save_setting_resume(void)
Change name to rk3288_restore_settings() to match rk3288_save_settings().


Line 348: flush_cache_all();
Maybe you should add a few more calls to flush_cache_all(), just in
case? ;) Just kidding.

Can you explain why you need so many calls? I would have expected that a
single flush_cache_all() would be enough (or maybe outer_disable() followed
by flush_cache_call()? I'm not an expert, but I think flush_cache_all()
goes to "Unconditionally clean and invalidate the entire cache."

Looking at other CPUs for inspiration:
* I only see one call to outer_flush_all() and it's for exynos and only if
CONFIG_CACHE_L2X0.
* There are some other outer_disable() callers, so maybe that's needed on
older rockchip boards with L2X0 (it shouldn't hurt to always call it).
That actually includes outer_flush_all() though.


Line 350: dsb();
: wfi();
: dsb();
Replace with a call to cpu_do_idle(), no?


Line 353: return 0;
nit: blank line before return.

Will you actually get end up here on a successful resume? Comparing with
exynos it looks like you only end up here if the wfi() failed. If the
wfi() succeeded you'll end up jumping to cpu_resume() eventually and that's
how you end up returning out. ...so they have an error message and
a "return 1" here.


Line 358: pr_info("%s\n", __func__);
remove


Line 372: pr_info("%s-out\n", __func__);
remove


Line 376: static int rk3288_suspend_iomap(void)
I might have to get some advice for how to do this properly, since I'm
pretty sure upstream won't like this.

...potentially we could just use iotable_init() to init "critical" sections
of memory like this. That's what things like exynos do.

You could probably leave this alone for now and when the patch is cleaner
we can get Heiko's advice.


Line 429: return ret;
nit: blank line before return.


Line 434: .valid = suspend_valid_only_mem,
nit: why tab after "valid"?


Line 437: static int rockchip_suspend_init_check
I don't think this function helps much. Just put it straight inside
rockchip_suspend_init().


Line 453: static struct rockchip_suspend_of_device_id
rockchip_suspend_dt_match[] = {
const


Line 461: {},
nit: nice if this looks like:

{ /* sentinel */ }

...to remind people not to delete it.


Line 464: int __init rockchip_suspend_init(void)
Return void. Right now you're always returning -1 and you're not checking
it in rockchip.c anyway.


Line 470: while (&matches->id && matches->init) {
The "&matches->id" makes no sense. Remove it and just loop
while "matches->init"


Line 475:
print an error if you get here?


https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 19: /* cpu sp addr */
nit: add some blank lines between things?


Line 22: #define RKPM_BOOTDATA_CPUCODE (RKPM_BOOTDATA_CPUSP+1)
I think this would be easier if things incremented by 4 and
included "_OFFSET" at the end?


Line 42: #define RKPM_BOOT_DATA_OFFSET
(RKPM_BOOT_CODE_OFFSET+RKPM_BOOT_CODE_SIZE)
nit: space around operators (like "+")

...same everywhere in this patch.


Line 73: struct rockchip_suspend_of_device_id {
Move this struct to "pm.c". It's only ever used in that file.


Line 74: struct of_device_id id;
All you care about is ".compatible" and ".data" and you don't pass this
structure to anyone who needs an of_device_id.

Why not just:

struct rockchip_suspend_of_device_id {
const char *compatible;
const struct platform_suspend_ops *ops;
int (*init)(void);
}


Line 88: /******fllowing is rk3288 defined**********/
s/fllowing/following


Line 132: pmu_pwr_mode_en = 0,
These are effectively constants. Make them CAPITAL?


Line 179: #endif
Please add a comment, like:

#endif /* !__ASSEMBLY__ */


Line 180: #endif
Add a comment:

#endif /* __MACH_ROCKCHIP_PM_H */


https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 25: 9: mov r1, r1
Why mov r1 to r1? This is just a nop, right?


Line 27: mrc p15, 0, r1, c0, c0, 5
Add comment saying that you're only going to continue with CPU 0.


Line 31: cpu1loop:
I guess other CPUs are ending up here? Do they all start back up at once?


Line 32: mov r3, #50
Why put 50 in r3?


Line 33: wfene
Why "ne"? Nothing is changing the condition code, so this is just wfe,
right? ...and it will never come out of this holding pen. Do you somehow
reset it to get it out?


Line 36: 1: mov r1, r1
Again, why move r1 to r1? This is just a nop...


Line 59: /*get SLP_DDR_NEED_RES ,if it is 1 ,ddr need to reusme*/
nits:
* blank line before
* space after ",", not before
* s/reusme/resume


Line 64: ldr r1, [r5, #(RKPM_BOOTDATA_DDRCODE*4)]
Is this code placed there by the bootloader? Do we have patches for that,
yet? Can you please point me to the code?


Line 69: ldr pc, [r5, #(RKPM_BOOTDATA_CPUCODE*4)]
nit: better to use blx? I don't think ldr handles thumb mode transitions
(just in case).

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>
Gerrit-HasComments: Yes

[Sonny Rao (Gerrit)]

Sonny Rao has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(8 comments)

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 42: RK3288_BOOTRAM_PHYS
this should probably get passed in through the DT? Similar to Kever's SMP
patch, or even just use the same properties from Kever's patch.


Line 72: date
nit: typo should be "data"


Line 140: &(B
nit: need spaces between "pwrmode" and "&" and "("


https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 170: #define RK3288_BOOTRAM_PHYS (0xFF720000)
: #define RK3288_BOOTRAM_SIZE SZ_4K
:
: #define RK3288_SGRF_PHYS (0xff740000)
: #define RK3288_SGRF_SIZE SZ_4K
Like I mentioned in the other file, I'm not sure if we should hardcode
these here necessarily or not.


https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/rockchip.c
File arch/arm/mach-rockchip/rockchip.c:

Line 32: rockchip_suspend_init();
should this only happen on rk3288 or all platforms?


https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 24: rockchip_slp_cpu_resume
I'm guessing this is a rk3288 specific function? Should it be named
rk3288_slp_cpu_resume?


Line 26: setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1 @ set svc, irqs off
Is it possible that we are in HYP mode at this point? Could we also clear
the CNTVOFF register prior to putting us into SVC mode, or just stay in HYP
mode?


Line 31: cpu1loop
maybe call this secondary_loop since it's not just CPU1 that will end up
here.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Sonny Rao (Gerrit)]

Sonny Rao has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 22: #define RKPM_BOOTDATA_CPUCODE (RKPM_BOOTDATA_CPUSP+1)
> I think this would be easier if things incremented by 4 and
> included "_OFFS

or maybe just a struct?

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Tony Xie (Gerrit)]

Tony Xie has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(40 comments)

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 42: rk_cpu_data.bootram_phy = (char *)RK3288_BOOTRAM_PHYS;

> this should probably get passed in through the DT? Similar to Kever's SMP

Done

Line 46: +RKPM_BOOT_CODE_OFFSET;
> nit: I think + goes on previous line.

Done

Line 69: memcpy(rdata->bootram_save, rdata->bootram_base,
> Does anyone else use this code? Do we really need to save / restore it,
> or

I want this code can use for all RK platform, there are two blocks of
internal memory in RK3288 SOC.one can be used for suspend/resume, anther is
used for booting secondary CPU. but in other RK SOC there is only one block
of internal memory.so it is needed to overlay the memory.


Line 72: /* move resume code and date to bootsram */

> nit: typo should be "data"

Done


Line 140: if (pwrmode&(BIT(pmu_scu_en)|BIT(pmu_a12_0_pd_en))) {

> nit: need spaces between "pwrmode" and "&" and "("

Done

Line 157: }
> I think that in both cases right now you're setting DDR_F to 0,
> right? ...

Done

Line 170: static u32 rk3288_slp_mode_set(int level)
> * Please add a comment for this function saying what it takes and what it
> r

Done

Line 184: readl_relaxed(rk3288_pmu_base+RK3288_PMU_PWRMODE_CON);
> Why do you need to keep the old values? Should only affect the next
> suspen

Done
Setting pmu low power mode and executing cpu wfi the system will enter
low power mode

Line 267: #define RK3288_CRU_SEL0_CON (0x60)
> What is so critical about these specific set of CRU registers? Is it
> possi

When system resume from low power mode,CPU will enter MSKROM first, CRU
registers will be changed by the code in MSKROM.so I have to resume it at
once

Line 279: * cru will be set in fastboot by ic,
> In "fastboot by ic"? What does this mean?

When system resume from low power mode,CPU will enter MSKROM first, then
enter fastboot.CRU registers will be changed by the code in MSKROM.so I
have to resume it at once.

I will edit "cru will be set in fastboot by ic," to "cru will be set in
mskrom code"

Line 296: writel_relaxed(0xff0f0010, rk3288_cru_base +
RK3288_CRU_MODE_CON);
> This mode doesn't make sense to me. The upper is the write mask, right?
> T

Done

I guess that means you're not changing the mode of DPLL. Why? Please
comment.

Yes,DPLL is for DDR, the code of this point is running in DDR, so DPLL can
not be set to enter 24m.

Line 301: writel_relaxed(rk3288_cru_mode | 0xffff0000,
> Shouldn't mode be last? I assume this will set PLLs back to fast mode but

the setting of plls speed not be changed.


Line 323: rk3288_powermode = rk3288_slp_mode_set(0);

> Add an enum for the possible values. So:

Done


Line 358: pr_info("%s\n", __func__);
> remove
Done


Line 372: pr_info("%s-out\n", __func__);
> remove
Done

Line 429: return ret;
> nit: blank line before return.

Done

Line 437: static int rockchip_suspend_init_check
> I don't think this function helps much. Just put it straight inside
> rockch

Done

Line 453: static struct rockchip_suspend_of_device_id
rockchip_suspend_dt_match[] = {
> const

Done

Line 461: {},
> nit: nice if this looks like:

Done

Line 464: int __init rockchip_suspend_init(void)
> Return void. Right now you're always returning -1 and you're not checking

Done


https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 42: #define RKPM_BOOT_DATA_OFFSET
(RKPM_BOOT_CODE_OFFSET+RKPM_BOOT_CODE_SIZE)
> nit: space around operators (like "+")

Done

Line 74: struct of_device_id id;
> All you care about is ".compatible" and ".data" and you don't pass this
> str

Done

Line 88: /******fllowing is rk3288 defined**********/
> s/fllowing/following

Done

Line 132: pmu_pwr_mode_en = 0,
> These are effectively constants. Make them CAPITAL?

Done


Line 174: #define RK3288_SGRF_SIZE SZ_4K

> Like I mentioned in the other file, I'm not sure if we should hardcode
> thes

RK3288_BOOTRAM_PHYS is necessary,because SGRF need to know which address
system boot from,when system resume.
I decide to get the register base and phy address from the rk3288 dtsi file.
pmu_intmem@ff720000 {
compatible = "mmio-sram";
reg = <0xff720000 0x4000>;
};
what is your suggestion?

Line 179: #endif
> Please add a comment, like:

Done

Line 180: #endif
> Add a comment:

Done

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/rockchip.c
File arch/arm/mach-rockchip/rockchip.c:

Line 32: rockchip_suspend_init();
> should this only happen on rk3288 or all platforms?

all platforms


https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 24: ENTRY(rockchip_slp_cpu_resume)

> I'm guessing this is a rk3288 specific function? Should it be named
> rk3288

I want this code can be used for all rockchip platform.

Line 25: 9: mov r1, r1
> Why mov r1 to r1? This is just a nop, right?

Done, a nop

Line 26: setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1 @ set svc, irqs off
> Is it possible that we are in HYP mode at this point? Could we also clear

HYP mode is used in arch64, so I am no idle for this question, can you
give me a suggestion.

Line 27: mrc p15, 0, r1, c0, c0, 5
> Add comment saying that you're only going to continue with CPU 0.

Done

Line 31: cpu1loop:
> maybe call this secondary_loop since it's not just CPU1 that will end up
> he

Done

Line 31: cpu1loop:
> I guess other CPUs are ending up here? Do they all start back up at once?

Done, the power domain of other CPUs is shut down in supend. so this code
is insure that these CPUs is in controlled state.

Line 32: mov r3, #50
> Why put 50 in r3?

Done
It is for debug,I will delet it.

Line 33: wfene
> Why "ne"? Nothing is changing the condition code, so this is just wfe,
> rig

Done.
Delete "mov r3, #50 ".
the reason of using"ne" is "cmp r1, #0" above.

Line 36: 1: mov r1, r1
> Again, why move r1 to r1? This is just a nop...

It is for debug,I will delete it.


Line 59: /*get SLP_DDR_NEED_RES ,if it is 1 ,ddr need to reusme*/
> nits:
Done

Line 64: ldr r1, [r5, #(RKPM_BOOTDATA_DDRCODE*4)]
> Is this code placed there by the bootloader? Do we have patches for that,

Done.
It is a binary file included in ddr.c in android project.but this project
the code for resuming DDR is not finish.so when the code is finish,I will
release it to you at once.

Line 69: ldr pc, [r5, #(RKPM_BOOTDATA_CPUCODE*4)]
> nit: better to use blx? I don't think ldr handles thumb mode transitions
> (

Done

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(1 comment)

I see a lot of "Done", but I don't see a new version of this patch.
Posting?

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 92: memcpy(rdata->bootram_save, rdata->bootram_base,
rdata->bootram_size);
As Julius pointed out in email, this is backward.

...as I pointed out above, this is probably not necessary.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Julius Werner (Gerrit)]

Julius Werner has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 69: memcpy(rdata->bootram_save, rdata->bootram_base,

> I want this code can use for all RK platform, there are two blocks of
> inte

Are you talking about blobs passed from firmware here? What firmware
specifically (U-Boot)?

The way I understand our currently plans, we hope to not need any blobs
that get passed down from coreboot for ChromeOS. Are there any other
bootloaders working differently (U-Boot?) that you are trying to preserve
compatibility for here? Otherwise, I think we can do away with this
entirely and just make it assume the firmware interface that coreboot
provides (which essentially is "we boot you on one core in secure SVC mode,
and then you're on your own").

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#2).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=None
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

5 files changed, 726 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 2

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#3).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=None
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

5 files changed, 757 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 3

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 69: memcpy(rdata->bootram_save, rdata->bootram_base,
> I want this code can use for all RK platform, there are two blocks of
> inte

No, smp use the other sram at 0xff700000. So maybe we needn't save /
restore this sram

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Kever Yang (Gerrit)]

Kever Yang has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 1:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/1/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 26: setmode PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1 @ set svc, irqs off
> Is it possible that we are in HYP mode at this point? Could we also clear

Sonny,
The CPU always reset as secure SVC mode by hardware design.
I think this code here is a setting to CPSR, which is able to set IRQ/FIQ,
change mode like SVC/USR/IRQ/FIQ/ABT/UNDEF, but we can't switch to HIGHER
level mode like MON and HYP, we need a 'SMC' command to switch to MON mode,
I think we can do that here to init CNTVOFF register.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 2:

(4 comments)

https://chromium-review.googlesource.com/#/c/215780/2/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 53: + RKPM_BOOT_CODE_OFFSET;
move + to the previous line


Line 184: readl_relaxed(rk3288_sgrf_base+RK3288_SGRF_SOC_CON0);
add spaces before and after '+'


Line 199: rk3288_pmupin_setfun(0x0, 0xa, 0x0, 0x0);
can you use pinctl for switch pin function


Line 215: | BIT(PMU_CHIP_PD_EN);
move the '|' up to the previous line

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 2

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 3:

(44 comments)

OK, enough comments for now. Hopefully you can spin this quickly and I'll
take another look at it...

https://chromium-review.googlesource.com/#/c/215780/3//COMMIT_MSG
Commit Message:

Line 10:
chrome-os-partner:32377


https://chromium-review.googlesource.com/#/c/215780/3/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 38: static u32 rk3288_bootram_phy;
nit: type should be "phys_addr_t"


Line 43: void rk_cpu_resume_data_init(void)
Just remove this function and the whole "rk_cpu_data" structure. It's just
as easy and clean to use the values directly in sram_data_for_sleep() and
sram_data_resume(), isn't it?


Line 48: rk_cpu_data.bootram_base = (char *)rk3288_bootram_base;
Either the variable shouldn't include "rk3288" in the name or the function
should include "rk3288" in the name.


Line 71: static void sram_data_for_sleep(struct rk_cpu_resume_data *rdata)
Let's put a verb in the name, like:

copy_data_to_sram

The other function could be:

restore_original_sram


Line 86: data_base = (unsigned long)rdata->bootram_base;
It seems hard to believe that this is necessary. When we jump to the code
the cache is totally turned off, isn't it? All addresses are physical in
any case. Any reason to believe we actually need this icache flush?


Line 101: data_base = (unsigned long)rdata->bootram_base;
Again, I'm not convinced this one is needed. On other Rockchip boards this
is used for SMP bringup, right? In that case the core is also running with
the cache off, right?


Line 145: static void rk3288_sram_data_prepare(u32 level)
The "prepare" makes it a little unclear whether this is configuring the
SRAM or doing something else. Maybe name it:

rk3288_fill_in_bootdata

?


Line 155: ((SZ_4K - 1) & ~0x7);
I think:

((SZ_4K - 1) & ~0x7)

Is equivalent to the (simpler):

(SZ_4K - 8)


Line 172: flush_cache_all();
You have a flush_cache_all() in rockchip_lpmode_enter(). I don't think you
need it here too, do you?


Line 232: mode_set |= BIT(PMU_SCU_EN) | BIT(PMU_CLK_CORE_SRC_GATE_EN);
PMU_SCU_EN is in both cases. Can it move above?


Line 260:
What do you think of?

/*
* A martian landed in my backyard and told me that if I
* didn't save and restore these registers right away he would
* eat my brain. That's why we do it here instead of adding
* some code into clk-rk3288.c registered with
* register_syscore_ops(). I like my brain.
*/
static const int saved_cru_reg_ids[] = {
RK3288_CRU_MODE_CON,
RK3288_CRU_SEL0_CON,
...
RK3288_CRU_SEL37_CON,
};
static saved_cru_regs[ARRAY_SIZE(saved_cru_reg_ids)];

Then the save code becomes:

for (i = 0; i < ARRAY_SIZE(saved_cru_reg_ids); i++) {
int reg_id = saved_cru_reg_ids[i]
saved_cru_regs[i] = readl_relaxed(rk3288_cru_base + reg_id);
}

...and restore code (running in the opposite order for symmetry):

for (i = ARRAY_SIZE(saved_cru_reg_ids) - 1; i >= 0; i--) {
int reg_id = saved_cru_reg_ids[i]
write_relaxed(saved_cru_regs[i] | 0xffff0000, rk3288_cru_base + reg_id);
}


Line 263: * so the operation is saving the set regs.
OK, sure. ...but is is _really_ critical that we do it here? If you add
some code using register_syscore_ops to clk-rk3288.c it will get executed
_pretty_ early. Are you sure you really need it to be here?

If you really really need it here, please justify why exactly, replacing my
snarky "martians made me do it" comment above. Something like:

The system starts out with lots of clock registers set back to
their default state. That means that we might be clocking things
totally wrong. It was OK to run like that during the MASKROM,
but the chip designer told me that if I don't set them correctly
within 1us of booting the kernel that my SDRAM would lose all
integrity. That's why we do it here instead of in a syscore callback
in the clock driver.

...or maybe:

The system is running at a paltry 2.3MHz right now. It's way
better to restore here instead of a syscore callback in the clock
code because otherwise things take __FOREVER__

...or:

At this point the clocks are something totally random and we
might be damaging the system. Sure, we were damaging it
during the MSKROM period too, but let's get out of the bad
state as soon as possible. We've calculated that you can get
100000 cycles of suspend resume without breaking the
system, so that should be good enough for anyone.


Specifically for the CRU (which is not a syscon) it would be nice to just
let the clock code handle it. Then we can totally avoid iomapping the cru,
right?


Line 282: * set plls into slow mode except dpll,
Why?

Why is it important that we put the PLLs in slow mode? ...and if it's
important, why is it _not_ important that we put the DPLL in slow mode?


Maybe this is only important for ROCKCHIP_ARM_OFF_LOGIC_NORMAL but not
important for ROCKCHIP_ARM_OFF_LOGIC_DEEP?


Line 285: writel_relaxed(RK3288_PLL_MODE_SLOW(RK3288_APLL_ID) |
Can't you use clk_set_parent if you really need to do this here?


Line 320: rk3288_slp_mode_set(level);
Unless you have a good reason not to, save and restore should be in
opposite order.

I think that means you should change this to:

rk3288_sram_data_prepare(level);
rk3288_slp_mode_set(level);
rk3288_cru_save();


Line 341:
Maybe print a warning here since you don't expect to get her normally
(right?)


Line 371: rk3288_pmu_base = of_iomap(node, 0);
Is there a reason you can't use the syscon API to access the PMU and GRF in
this code?


Line 430: /*
I'll repeat my comment from patch set #1, since it wasn't responded to:

Wait, what are you doing here? Changing GPIO0_A0 to a GPIO? Shouldn't it
be "function 1" so that we can communicate to the PMIC that we're turning
off?
Also: I don't think this belongs here. Can't we just set the proper
function through a pinctrl "hog"? ...or is there some reason you need to do
this _right here_? If we can remove this we can remove the whole
rk3288_pmupin_setfun().

Line 442: .valid = suspend_valid_only_mem,
Again, why is there a tab after "valid" but a space after "enter"


Line 445: static struct rockchip_suspend_of_device_id
rockchip_suspend_dt_match[] = {
Again. "const" ...actually, "initconst", so:

static const struct rockchip_suspend_of_device_id
rockchip_suspend_dt_match[] __initconst = {


Line 457: struct rockchip_suspend_of_device_id *matches =
Try:

const struct rockchip_suspend_of_device_id *matches


Line 460: while (&matches->compatible && matches->ops) {
The "&matches->compatible" is just as silly as "&matches->id" from patch
set #1.

* Let's say that "matches" starts out as 0x10000000.
* On the first iteration, "&matches->compatible" is exactly equal to
0x10000000, right? That's because "compatible" is the first entry in the
struct.
* On the second iteration, you've done "matches++". Now "matchse" is
0x1000000c (the structure is 0xc bytes big, right)? ...and of
course "&matches->compatible" is also 0x1000000c.

...as you can probably guess, "&matches->compatible" will never be NULL.
There's no reason to check for it.


Line 461: if (of_machine_is_compatible(matches->compatible)) {
nit: reduce indentation by doing:

if (!of_machine_is_compatible(matches->compatible))
continue;


Line 467: break;
Just return?


Line 471: ret = 0;
Shouldn't you just return right away? Why keep searching?


Line 476: pr_err("%s:there is not a machine matched\n", __func__);
nit: space after ":"

...if you get here you should print an error no matter what. No need to
check "ret"


https://chromium-review.googlesource.com/#/c/215780/3/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 13: *
Remove extra line with just a "*"


Line 20: #define RKPM_BOOTDATA_CPUSP (0)

nit: add some blank lines between things?

...but see "sleep.S" and I'm suggesting we move to just declaring variables.


Line 26: #define RKPM_BOOTDATA_DPLL_F (RKPM_BOOTDATA_DDR_F + 1)
You don't use RKPM_BOOTDATA_DPLL_F...


Line 32: #define RKPM_BOOTDATA_L2LTY_F (RKPM_BOOTDATA_DDRDATA + 1)
Could you put "L2CTLR" somewhere in the name instead of "L2LTY"? Then it
matches with the ARM TRM.


Line 49: char *bootram_base;
These should be "void *", not "char *"

...but in "pm.c" I'm suggesting you kill this whole structure anyway.


https://chromium-review.googlesource.com/#/c/215780/3/arch/arm/mach-rockchip/rockchip.c

File arch/arm/mach-rockchip/rockchip.c:

Line 32: rockchip_suspend_init();

There's a merge conflict here on cpufreq, so you need a rebase...


https://chromium-review.googlesource.com/#/c/215780/3/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 21: .data
Add a comment explaining why this is in the data section

Line 25: 9: mov r1, r1

Replace with "nop". ...and add a comment about why you need a nop here.


Line 30: /* olny cpu0 can continue to run,the others is halt here*/
nit: space after ","


Line 33: wfene
You don't need the "ne" still. If the "cmp r1, #0" showed that we were cpu
0 we would have branched to cpu0run. Just make this "wfe".


Line 38: /* resume data offset, from ram base */
nit: only one space between "data" and "offset"


Line 39: ldr r5, 8f
How about:

ldr r5, =(RKPM_BOOT_CODE_OFFSET + RKPM_BOOT_CODE_SIZE)

...actually all of your calculations are all based upon the start of
BOOT_CODE (label 9 above), so you really shouldn't be adding the
RKPM_BOOT_CODE_OFFSET. In other words, this is more correct as:

ldr r5, =RKPM_BOOT_CODE_SIZE

...but RKPM_BOOT_CODE_SIZE can fit in an immediate, so even better is:

mov r5, #RKPM_BOOT_CODE_SIZE

...but then you can also combine with the add:

add r5, r1, #RKPM_BOOT_CODE_SIZE

...or, you can see the comment at the end of this file and we can get rid
of all of these calculations.


Line 49: arm_errata__818325:
This might be a lot more palatable to upstream if we didn't call it
errata__818325. Do you know the name of this register? If you do just
name the register here and leave out the word "errata".


Line 53: ldr r3 , [r5, #(RKPM_BOOTDATA_ARM_ERRATA_818325 * 4)]
nit: no space before ","


Line 58: /* get SLP_DDR_NEED_RES, if it is 1, ddr need to resume*/
In this case I think you could probably just check if [r5,
#(RKPM_BOOTDATA_DDRCODE * 4)] is non-zero. I think you can be guaranteed
that the address of the code is not 0x00000000.


Line 69: blx r1
My mistake, this should be "bx r1" instead of "blx r1". No need to set the
link register.


Line 71: ENDPROC(rockchip_slp_cpu_resume)
Let's make all our lives a whole lot easier cleaner. We can just declare
all the variables here in the assembly:

/* Parameters filled in by the kernel */

/* 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

/* Code to jump to for DDR resume, or NULL */
.global rkpm_bootdata_ddr_code
rkpm_bootdata_ddr_code:
.long 0

/* Data to pass to DDR resume code */
.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

/* Flag for whether to restore UNKNOWN_REGISTER on resume */
.global rkpm_bootdata_unknown_reg_f
rkpm_bootdata_unknown_reg_f:
.long 0

/* Saved UNKNOWN_REGISTER to restore on resume */
.global rkpm_bootdata_unknown_reg
rkpm_bootdata_unknown_reg:
.long 0

Once you do that you can access them like anything else in assembly.
In "pm.h" in the "not ASSEMBLY" part:

extern u32 *rkpm_bootdata_cpusp;
extern phys_addr_t rkpm_bootdata_cpu_code;
extern phys_addr_t rkpm_bootdata_ddr_code;
extern phys_addr_t rkpm_bootdata_ddr_data;
extern bool rkpm_bootdata_l2ctlr_f;
extern u32 rkpm_bootdata_l2ctlr;
extern bool rkpm_bootdata_unknown_reg_f;
extern u32 rkpm_bootdata_unknown_reg;

Now you can access the stuff in C too!

Once you've done those two things, now the "data" and the "code" can be
just considered a single chunk.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 3

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#4).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377

TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---

M arch/arm/boot/dts/rk3288-pinky-rev1.dts
M arch/arm/boot/dts/rk3288.dtsi

M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

7 files changed, 590 insertions(+), 1 deletion(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>

Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 3:

(2 comments)

https://chromium-review.googlesource.com/#/c/215780/3//COMMIT_MSG
Commit Message:

Line 10:
> chrome-os-partner:32377

Done

https://chromium-review.googlesource.com/#/c/215780/3/arch/arm/mach-rockchip/rockchip.c
File arch/arm/mach-rockchip/rockchip.c:

Line 32: rockchip_suspend_init();
> There's a merge conflict here on cpufreq, so you need a rebase...

Done

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 3

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 4:

(1 comment)

Doh. I only saw a few things marked "Done" in my email so I didn't even
look at this review today. Sorry!

I will try to go through a full review tomorrow...

https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/boot/dts/rk3288-pinky-rev1.dts
File arch/arm/boot/dts/rk3288-pinky-rev1.dts:

Line 327: pinctrl-0 = <&pmic_int &global_pwroff>;
Should change pinky in a separate CL. to facilitate upstreaming.

...also, I would be tempted (maybe?) to say that the "global_pwroff" should
be done in a pinctrl "hog". I'll try to find the exact syntax but
effectively I think you add the pinctrl setting to the pinctrl node itself.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Derek Basehore (Gerrit)]

Derek Basehore has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 4:

Can we get a patch that adds in the rk3288-pmu-sram compatible node to the
device tree?

This is currently missing from our tree, so this can't be tested on our end.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: No

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 4:

(12 comments)

Wow, this is getting _so_ much cleaner!

...a few more changes...

https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/boot/dts/rk3288-pinky-rev1.dts
File arch/arm/boot/dts/rk3288-pinky-rev1.dts:

Line 327: pinctrl-0 = <&pmic_int &global_pwroff>;
> Should change pinky in a separate CL. to facilitate upstreaming.

You know what, let's leave it here (don't use a hog). Still should be a
separate CL, though.


https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/boot/dts/rk3288.dtsi
File arch/arm/boot/dts/rk3288.dtsi:

Line 680: sleep {
You'd need to get Heiko's advice to be certain, but I think that it might
be handy if the dtsi change happens in a separate patch (since it may go
through a different tree).


https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 72: rkpm_bootdata_l2ctlr = rk3288_l2_config();
nit: one space before "="


Line 142: static inline void rk3288_slp_mode_set_resume(void)
* is "inline" required for functionality? If not, remoe it
* nit: remove double-space after "void"


Line 166: * The apll/cpll/gpll will be set into slow mode in maskrom.
Have you measured how much slower resume happens if you do this as a
syscore_op in clk-rk3288.c? If resume takes <50ms longer then we should
just do it there.


Line 180: }
I guess it wasn't important to actually transition the PLLs into slow mode
yourself? Nice!


Line 329: if (of_machine_is_compatible(matches->compatible)) {
Again.

nit: reduce indentation by doing:

if (!of_machine_is_compatible(matches->compatible))
continue;

...or respond to my comment and tell me why you didn't.


Line 346: return;
Remove. return is implicit here.


https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/pm.h

File arch/arm/mach-rockchip/pm.h:

Line 13: *

Again.

Remove extra line with just a "*"

https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 22: * this code will be copide from
nit: line up the "*"s

nit: s/copide/copied


Line 39: adr r2, rkpm_bootdata_l2ctlr_f
nit: I think there's no need for separate "adr" and "ldr".

ldr r2, rkpm_bootdata_l2ctlr_f

Right?


Line 41: cmp r3, #1
Don't you need a branch somewhere? You're loading the flag but not paying
attention to it. In other words you need a branch to "sp_set" somewhere,
right?

Also, it's slightly cleaner to compare to 0. AKA:

cmp r3, #0
beq sp_set

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 4:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 166: * The apll/cpll/gpll will be set into slow mode in maskrom.
> Have you measured how much slower resume happens if you do this as a
> syscor

...and the burden of proof is on you. In other words: if you aren't setup
to measure this accurately (by twiddling a GPIO or something) and the
difference in time is not user perceptible then you should move the code to
clk-rk3288.c until you can prove that it needs to be here. It should only
be here if you have real data showing that it has to be here.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Heiko Stübner (Gerrit)]

Heiko Stübner has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 4:

(8 comments)

https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/boot/dts/rk3288.dtsi
File arch/arm/boot/dts/rk3288.dtsi:

Line 680: sleep {
> You'd need to get Heiko's advice to be certain, but I think that it might
> b

Correct.

While both the mach-rockchip code and dts changes would go through arm-soc,
the arm-soc maintainers like to have separate branches for these soc and
dts changes.


https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 133: */
nit: one space more before "*/"

Line 166: * The apll/cpll/gpll will be set into slow mode in maskrom.

> ...and the burden of proof is on you. In other words: if you aren't setup

An initial upstream submission should in any case do this in the clock
driver.

Also why are only parts of the registers saved (mode, sel0, sel1, sel10,
sel33, sel37)? While these contain core clocks, I don't see why you can get
away with not saving the others which may get their dividers/muxes reset in
the maskrom when waking up too.

For example Samsung saves all relevant clock registers.


Line 313: rockchip_suspend_dt_match[] __initconst = {
nit: please move this "rockchip_suspend_dt_match..." a bit more to the
right.
Kernel coding styling suggests continuing lines to be at the right-most
position. As it is now it more suggests an error with the parenthese on the
next line, making it harder to read.


Line 344: pr_err("%s: there is not a machine matched\n", __func__);
nit: not sure if this error is necessary


https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 20: #define RKPM_BOOT_CODE_SIZE (0x200)
where does this 0x200 size come from? If it's the real size of the code you
want to copy to sram, this can be calculated from the real code (see how
the smp does this).

Hardcoding the size may produce issues if the sram code grows beyond this
and nobody notices.


Line 39: #define RKPM_BITS_CLR_VAL(val, bits_shift, msk)
(val&~(msk<<bits_shift))
please don't add such macros, it obscures readability of the code.

[also the formatting is wrong (spaces and such)]


Line 46: #define RK3288_PMU_WAKEUP_CFG0 0x00
This is a local header, please only define stuff you're actually using.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 4:

Ping?

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: No

[Tony Xie (Gerrit)]

Tony Xie has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 4:

(14 comments)

https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 72: rkpm_bootdata_l2ctlr = rk3288_l2_config();
> nit: one space before "="

Done

Line 133: */
> nit: one space more before "*/"

Done

Line 142: static inline void rk3288_slp_mode_set_resume(void)
> * is "inline" required for functionality? If not, remoe it

Done

Line 166: * The apll/cpll/gpll will be set into slow mode in maskrom.

> Have you measured how much slower resume happens if you do this as a
> syscor

Done

Line 313: rockchip_suspend_dt_match[] __initconst = {
> nit: please move this "rockchip_suspend_dt_match..." a bit more to the
> righ

Done

Line 329: if (of_machine_is_compatible(matches->compatible)) {
> Again.

Done

Line 346: return;
> Remove. return is implicit here.

Done


https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 13: *
> Again.
Done

Line 20: #define RKPM_BOOT_CODE_SIZE (0x200)
> where does this 0x200 size come from? If it's the real size of the code
> you

Done


Line 39: #define RKPM_BITS_CLR_VAL(val, bits_shift, msk)
(val&~(msk<<bits_shift))

> please don't add such macros, it obscures readability of the code.

Done

Line 46: #define RK3288_PMU_WAKEUP_CFG0 0x00
> This is a local header, please only define stuff you're actually using.

Done

https://chromium-review.googlesource.com/#/c/215780/4/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 22: * this code will be copide from
> nit: line up the "*"s

Done

Line 39: adr r2, rkpm_bootdata_l2ctlr_f
> nit: I think there's no need for separate "adr" and "ldr".

Done

Line 41: cmp r3, #1
> Don't you need a branch somewhere? You're loading the flag but not paying

Done

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 4

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#5).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377

TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---

M arch/arm/boot/dts/rk3288-pinky-rev1.dts
M arch/arm/boot/dts/rk3288-pinky-rev2.dts
M arch/arm/boot/dts/rk3288.dtsi

M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

M drivers/clk/rockchip/clk-rk3288.c
9 files changed, 560 insertions(+), 3 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 5

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Julius Werner (Gerrit)]

Julius Werner has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 5:

(1 comment)

Maybe this slipped my mind, but what was the latest state of the DDR reinit
part? Is someone working on that in parallel? I would really like to see at
least an early WIP version so we can get an idea how it looks and how it
would fit into this.

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 25: */
nit: I don't quite see why this is necessary. Couldn't you just as well
copy it out of .text? I think putting it in .text or .rodata would map it
read-only which is a nice security benefit.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 5

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 5:

(8 comments)

Keeps looking better! :) A few more issues...

---

I think this is about ready to be submitted upstream. For upstream, this
will need to be several patches, like:

1. Patch #1 add save/restore of clocks
2. Patch #2: device tree bindings
3. Patch #3: pm.c, pm.h, rockchip.c, sleep.s, Makefile
4. Patch #4: rk3288.dtsi
5. Patch #5: EVB

...there's no pinky dts upstream yet, so that patch will just be a local
patch (not submitted upstream).

---

We'd like this to at least be submitted upstream before we pick it locally.

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/boot/dts/rk3288.dtsi
File arch/arm/boot/dts/rk3288.dtsi:

Line 483: compatible = "mmio-sram", "rockchip,rk3288-pmu-sram";
This needs to be documented somewhere in Documentation/devicetree/bindings.


https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 132: * arm off,logic normal
nit: Please line up stars


Line 274: continue;
Now that I see it written this way, it's obvious to me what the clean way
should be. Sorry for not suggesting earlier.

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);


https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 19: #define RKPM_BOOT_CODE_SIZE (0x200)
Remove this constant since it's no longer used.

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 25: */
> nit: I don't quite see why this is necessary. Couldn't you just as well
> cop

Good point.


Line 50: adr r0, rkpm_bootdata_ddr_data
I think this should be an ldr, right? You want to pass a pointer, not a
pointer to a pointer...


https://chromium-review.googlesource.com/#/c/215780/5/drivers/clk/rockchip/clk-rk3288.c
File drivers/clk/rockchip/clk-rk3288.c:

Line 769: RK3288_MODE_CON,
From patch set #4 (in pm.c), Heiko asked:

> Also why are only parts of the registers saved
> (mode, sel0, sel1, sel10, sel33, sel37)? While these
> contain core clocks, I don't see why you can get
> away with not saving the others which may get their
> dividers/muxes reset in the maskrom when waking
> up too.

> For example Samsung saves all relevant clock registers.

Can you answer his question?


Line 865: register_syscore_ops(&rk3288_clk_syscore_ops);
I think this needs to be around CONFIG_PM_SLEEP. To be a little less ugly,
you should probably create "rk3288_clk_sleep_init" like Samsung did and
define as "static void rk3288_clk_sleep_init(void) {}" if no PM_SLEEP.
Then call it here.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 5

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 5:

(2 comments)

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/boot/dts/rk3288-pinky-rev2.dts
File arch/arm/boot/dts/rk3288-pinky-rev2.dts:

Line 380: regulator-suspend-mem-microvolt = <1000000>;
Why is this not "regulator-suspend-mem-disabled;"?


Line 387: regulator-suspend-mem-enable;
missing a "d"

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 5

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 5:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 26: #include <asm/suspend.h>
Please sort <> includes alphabetically and by group:

#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 <asm/cacheflush.h>
#include <asm/tlbflush.h>
#include <asm/suspend.h>

#include "pm.h"

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 5:

I think we want to squash
<https://chromium-review.googlesource.com/#/c/222940/> into this.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 5
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: No

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 5:

@julius
Yes, right now we don't have the ddr reinit code, so pd_bus still enable
when enter suspend. tony and hcy is working for it.
@tony, I think Doug want we to send this patch to upstream, after you
modify the patch. And when we finish the ddr reinit code, add it to the
upstream patches, maybe v2 or v3.

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 5:

Right. I think we could post this upstream for comments even before the
DDR init code is ready. ...but if it's going to be ready in the next day
or two we could wait for it.

I was hoping to see a new version of this patch today that addresses
comments. Is it ready?

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 5:

I think hcy need more time to finish the ddr reinit. I'll help Tony sort
the code for upstream.

[Julius Werner (Gerrit)]

Julius Werner has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 5:

Hi Chris,

Can you ask hcy to upload a work-in-progress version of what he has for DDR
reinit right now? It's okay if it doesn't work yet, I just want to get a
rough idea of how far it is along and how it will look. Adding some early
comments to an unfinished version could save us time reviewing it later.

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#6).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---
M arch/arm/boot/dts/rk3288-pinky-rev1.dts
M arch/arm/boot/dts/rk3288-pinky-rev2.dts
M arch/arm/boot/dts/rk3288.dtsi
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
M drivers/clk/rockchip/clk-rk3288.c

9 files changed, 569 insertions(+), 3 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 6

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 5:

(11 comments)

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/boot/dts/rk3288-pinky-rev2.dts
File arch/arm/boot/dts/rk3288-pinky-rev2.dts:

Line 380: regulator-suspend-mem-microvolt = <1000000>;
> Why is this not "regulator-suspend-mem-disabled;"?

It doesn't support cut off vdd_gpu right now. It will cause crash, Let me
check with Tony and Simon, I will add it to here once we solve this problem

Line 387: regulator-suspend-mem-enable;
> missing a "d"

Done

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/boot/dts/rk3288.dtsi
File arch/arm/boot/dts/rk3288.dtsi:

Line 483: compatible = "mmio-sram", "rockchip,rk3288-pmu-sram";
> This needs to be documented somewhere in
> Documentation/devicetree/bindings.

Done

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 26: #include <asm/suspend.h>
> Please sort <> includes alphabetically and by group:

Done

Line 132: * arm off,logic normal
> nit: Please line up stars

Done

Line 274: continue;
> Now that I see it written this way, it's obvious to me what the clean way
> s

Done

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/pm.h
File arch/arm/mach-rockchip/pm.h:

Line 19: #define RKPM_BOOT_CODE_SIZE (0x200)
> Remove this constant since it's no longer used.

Done

https://chromium-review.googlesource.com/#/c/215780/5/arch/arm/mach-rockchip/sleep.S
File arch/arm/mach-rockchip/sleep.S:

Line 25: */

> Good point.
it is not read only, rkpm_bootdata_cpu_code will be overwrite in pm.c

Line 50: adr r0, rkpm_bootdata_ddr_data
> I think this should be an ldr, right? You want to pass a pointer, not a
> po

Done

https://chromium-review.googlesource.com/#/c/215780/5/drivers/clk/rockchip/clk-rk3288.c
File drivers/clk/rockchip/clk-rk3288.c:

Line 769: RK3288_MODE_CON,
> From patch set #4 (in pm.c), Heiko asked:

this still need tony confirmation, but I do it first.

Line 865: register_syscore_ops(&rk3288_clk_syscore_ops);
> I think this needs to be around CONFIG_PM_SLEEP. To be a little less
> ugly,

Done

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 5

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 6:

(5 comments)

https://chromium-review.googlesource.com/#/c/215780/6/arch/arm/boot/dts/rk3288.dtsi

File arch/arm/boot/dts/rk3288.dtsi:

Line 483: compatible = "mmio-sram", "rockchip,rk3288-pmu-sram";

I don't see anything in Documentation/devicetree/bindings in this patch.
Did you forget to add?


https://chromium-review.googlesource.com/#/c/215780/6/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 22: #include <linux/regulator/machine.h>
I think usually things with subdirectories get sorted at the end.


https://chromium-review.googlesource.com/#/c/215780/6/drivers/clk/rockchip/clk-rk3288.c
File drivers/clk/rockchip/clk-rk3288.c:

Line 769: #define RK3288_CLKSEL_NUM 43
It _might_ still make sense to add things one at a time so we can pick and
choose (and also make sure we get the right order).

I've always been a little skeptical about Samsung's approach here of just
jamming everything in since you need to be of the ordering between
dividers, rates, muxes, etc.

I would tend to keep the old code for now (just 0, 1, 10, 33, 37) and think
about a better solution...


Line 770: static u32 rk3288_saved_cru_regs[RK3288_CLKSEL_NUM + 1];
Why + 1?


Line 823: static void __init rk3288_clk_init(struct device_node *np)
nit: blank line

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 6

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 6:

(5 comments)

https://chromium-review.googlesource.com/#/c/215780/6/arch/arm/boot/dts/rk3288.dtsi
File arch/arm/boot/dts/rk3288.dtsi:

Line 483: compatible = "mmio-sram", "rockchip,rk3288-pmu-sram";
> I don't see anything in Documentation/devicetree/bindings in this patch.
> D

Ah, forgot to upload it

https://chromium-review.googlesource.com/#/c/215780/6/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 22: #include <linux/regulator/machine.h>
> I think usually things with subdirectories get sorted at the end.

Done

https://chromium-review.googlesource.com/#/c/215780/6/drivers/clk/rockchip/clk-rk3288.c
File drivers/clk/rockchip/clk-rk3288.c:

Line 769: #define RK3288_CLKSEL_NUM 43
> It _might_ still make sense to add things one at a time so we can pick and

yes, I think we just need to restore the basic clk, the ones would be
changed in suspend. 0: is A17 clk, 1: pd_bus, ...

Line 770: static u32 rk3288_saved_cru_regs[RK3288_CLKSEL_NUM + 1];
> Why + 1?

add a RK3288_MODE_CON

Line 823: static void __init rk3288_clk_init(struct device_node *np)
> nit: blank line

Done

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#7).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---

A Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt

M arch/arm/boot/dts/rk3288-pinky-rev1.dts
M arch/arm/boot/dts/rk3288-pinky-rev2.dts
M arch/arm/boot/dts/rk3288.dtsi
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
M drivers/clk/rockchip/clk-rk3288.c

10 files changed, 585 insertions(+), 3 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 7

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#8).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---
A Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt
M arch/arm/boot/dts/rk3288-pinky-rev1.dts
M arch/arm/boot/dts/rk3288-pinky-rev2.dts
M arch/arm/boot/dts/rk3288.dtsi
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
M drivers/clk/rockchip/clk-rk3288.c

10 files changed, 586 insertions(+), 3 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 8

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 8:

now we can shut down the vdd_gpu, with below patches,
https://chromium-review.googlesource.com/#/c/215430/
https://chromium-review.googlesource.com/#/c/215432/9
https://chromium-review.googlesource.com/#/c/215433/9
https://chromium-review.googlesource.com/#/c/223464/

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 8
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: No

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 8:

(2 comments)

https://chromium-review.googlesource.com/#/c/215780/8/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 257: };
Still no prepare / finish?


https://chromium-review.googlesource.com/#/c/215780/8/drivers/clk/rockchip/clk-rk3288.c
File drivers/clk/rockchip/clk-rk3288.c:

Line 795: __raw_readl(rk3288_cru_base + reg_id);
Why __raw_readl and not readl_relaxed?

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 8
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 8:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/8/arch/arm/mach-rockchip/Makefile
File arch/arm/mach-rockchip/Makefile:

Line 3: obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
Alphabetize?

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 8:

(3 comments)

https://chromium-review.googlesource.com/#/c/215780/8/arch/arm/mach-rockchip/Makefile
File arch/arm/mach-rockchip/Makefile:

Line 3: obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
> Alphabetize?

Done

https://chromium-review.googlesource.com/#/c/215780/8/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 257: };
> Still no prepare / finish?

ok, I will move them to this patch

https://chromium-review.googlesource.com/#/c/215780/8/drivers/clk/rockchip/clk-rk3288.c
File drivers/clk/rockchip/clk-rk3288.c:

Line 795: __raw_readl(rk3288_cru_base + reg_id);
> Why __raw_readl and not readl_relaxed?

I made reference to the suspend code in samsumg. is it same? maybe
readl_relaxed is better. I will change it back

[Chris Zhong (Gerrit)]

Hello Tony Xie,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#9).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---
A Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt
M arch/arm/boot/dts/rk3288-pinky-rev1.dts
M arch/arm/boot/dts/rk3288-pinky-rev2.dts
M arch/arm/boot/dts/rk3288.dtsi
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
M drivers/clk/rockchip/clk-rk3288.c

10 files changed, 598 insertions(+), 3 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 9

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

Patch Set 9: Code-Review+1

(1 comment)

This is looking pretty nice. Should we split it up and add FROMLIST where
appropriate?

https://chromium-review.googlesource.com/#/c/215780/9/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 209: regulator_suspend_finish();
Since this returns an error code, should probably at least print a warning
if it fails?

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 9
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: Yes

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................


Patch Set 9:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/9/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 209: regulator_suspend_finish();
> Since this returns an error code, should probably at least print a warning

Done

[Chris Zhong (Gerrit)]

Hello Tony Xie, Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#10).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---
A Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt
M arch/arm/boot/dts/rk3288-pinky-rev1.dts
M arch/arm/boot/dts/rk3288-pinky-rev2.dts
M arch/arm/boot/dts/rk3288.dtsi
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
M drivers/clk/rockchip/clk-rk3288.c

10 files changed, 620 insertions(+), 3 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 10

[Derek Basehore (Gerrit)]

Hello Tony Xie, Chris Zhong, Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#11).

Change subject: suspend: rockchip: add suspend and resume of rk3288
......................................................................

suspend: rockchip: add suspend and resume of rk3288

This adds support for Rockchip soc suspend & resume

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Chris.Zhong <z...@rock-chips.com>
Signed-off-by: Tony.xie <tony...@rock-chips.com>
---

M arch/arm/boot/dts/rk3288-pinky-rev1.dts

M arch/arm/boot/dts/rk3288.dtsi
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

7 files changed, 590 insertions(+), 1 deletion(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 11

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#12).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/rockchip.c
...due to local patches in that file.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>
Signed-off-by: Doug Anderson <dian...@chromium.org>
(am from https://patchwork.kernel.org/patch/5128821/)
---

M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

5 files changed, 499 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 12

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#13).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/rockchip.c
...due to local patches in that file.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>

Reviewed-by: Doug Anderson <dian...@chromium.org>
Tested-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>
(am from https://patchwork.kernel.org/patch/5160821/)

---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

5 files changed, 501 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 13

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

Patch Set 13: Code-Review-1

Should finish addressing Kevin's feedback upstream.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 13
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: No

[Chris Zhong (Gerrit)]

Hello Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#14).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/rockchip.c
...due to local patches in that file.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>
Reviewed-by: Doug Anderson <dian...@chromium.org>
Tested-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>
(am from https://patchwork.kernel.org/patch/5160821/)
---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
5 files changed, 501 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 14

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#15).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/rockchip.c
...due to local patches in that file.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>

Tested-by: Doug Anderson <dian...@chromium.org>
Reviewed-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>
(am from https://patchwork.kernel.org/patch/5186941/)

---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

5 files changed, 474 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 15

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

Patch Set 15: Code-Review-1

Doesn't actually work without FIXUP. See the next patch. This is the
newest version that's been sent upstream, though there is review feedback
upstream, too.

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 15
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-HasComments: No

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#16).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/rockchip.c
...due to local patches in that file.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>
Tested-by: Doug Anderson <dian...@chromium.org>
Reviewed-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>
(am from https://patchwork.kernel.org/patch/5186941/)
---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
5 files changed, 474 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 16

[Chris Zhong (Gerrit)]

Hello Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#17).

Gerrit-PatchSet: 17

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#18).

Gerrit-PatchSet: 18

[Daniel Kurtz (Gerrit)]

Hello Chris Zhong, Doug Anderson, Dominik Behr, 智情 姚,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#19).

Gerrit-PatchSet: 19

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>

Gerrit-Reviewer: Daniel Kurtz <djk...@chromium.org>

Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>

Gerrit-Reviewer: Dominik Behr <db...@chromium.org>

Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#20).

Gerrit-PatchSet: 20

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#21).

Gerrit-PatchSet: 21

[Chris Zhong (Gerrit)]

Hello Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#22).

Gerrit-PatchSet: 22

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#23).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:

arch/arm/mach-rockchip/Makefile
arch/arm/mach-rockchip/rockchip.c
...conflicts due to local l2 cache code (we skipped some upstream
cleanup commits) and due to power domain patches.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>
Tested-by: Doug Anderson <dian...@chromium.org>
Reviewed-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>

(am from https://patchwork.kernel.org/patch/5252831/)

---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S

5 files changed, 439 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 23

[Sonny Rao (Gerrit)]

Hello Chris Zhong, Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#24).

Gerrit-PatchSet: 24

[王 晓腾 (Gerrit)]

Hello Chris Zhong, Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#25).

Gerrit-PatchSet: 25

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Daniel Kurtz <djk...@chromium.org>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Dominik Behr <db...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>
Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>

Gerrit-Reviewer: 王 晓腾 <caesa...@rock-chips.com>

[Sonny Rao (Gerrit)]

Hello Chris Zhong, Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#26).

Gerrit-PatchSet: 26

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#27).

Gerrit-PatchSet: 27

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#28).

Gerrit-PatchSet: 28

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#29).

Gerrit-PatchSet: 29

[Daniel Kurtz (Gerrit)]

Hello Chris Zhong, Doug Anderson,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#30).

Gerrit-PatchSet: 30

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#31).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/Makefile
arch/arm/mach-rockchip/rockchip.c
...conflicts due to local l2 cache code (we skipped some upstream
cleanup commits) and due to power domain patches.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>
Tested-by: Doug Anderson <dian...@chromium.org>
Reviewed-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>

(am from https://patchwork.kernel.org/patch/5311641/)

---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
5 files changed, 439 insertions(+), 0 deletions(-)


--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17

Gerrit-PatchSet: 31

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

Patch Set 31: Code-Review+1 Verified+1

(1 comment)

I'm not opposed to landing this and taking fixups, but I _think_ the mali
patch isn't quite ready yet and I think this doesn't work without the mali
patch, so maybe we can continue to wait on upstream feedback?

Happy to take other opinions.

https://chromium-review.googlesource.com/#/c/215780/31/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 190: rk3288_config_bootdata();
You ignored my comments upstream (in v7) that the config_bootdata and the
memcpy() no longer really belong in the iomap function...

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 31
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Daniel Kurtz <djk...@chromium.org>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Dominik Behr <db...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>
Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>
Gerrit-Reviewer: 王 晓腾 <caesa...@rock-chips.com>

Gerrit-HasComments: Yes

[Chris Zhong (Gerrit)]

Chris Zhong has posted comments on this change.

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................


Patch Set 31:

(1 comment)

https://chromium-review.googlesource.com/#/c/215780/31/arch/arm/mach-rockchip/pm.c
File arch/arm/mach-rockchip/pm.c:

Line 190: rk3288_config_bootdata();
> You ignored my comments upstream (in v7) that the config_bootdata and the
> m

Ah, if I the v8 is required, I will modify it.

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#32).

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/Makefile
arch/arm/mach-rockchip/rockchip.c
...conflicts due to local l2 cache code (we skipped some upstream
cleanup commits) and due to power domain patches.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>
Tested-by: Doug Anderson <dian...@chromium.org>
Reviewed-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>
(am from https://patchwork.kernel.org/patch/5311641/)
---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
5 files changed, 439 insertions(+), 0 deletions(-)

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 32

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#33).

Gerrit-PatchSet: 33

[Doug Anderson (Gerrit)]

Hello Chris Zhong,

I'd like you to reexamine a change. Please visit

https://chromium-review.googlesource.com/215780

to look at the new patch set (#34).

Gerrit-PatchSet: 34

Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Daniel Kurtz <djk...@chromium.org>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Dominik Behr <db...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>

Gerrit-Reviewer: chrome-internal-fetch <chrome-int...@google.com>

[Doug Anderson (Gerrit)]

Doug Anderson has posted comments on this change.

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................

Patch Set 34: Code-Review+2 Commit-Queue+1 Verified+1

Let's take it and we can take fixups later if upstream does something
different. ...or we diverge...

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 34
Gerrit-Project: chromiumos/third_party/kernel
Gerrit-Branch: chromeos-3.14
Gerrit-Owner: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Chris Zhong <z...@rock-chips.com>
Gerrit-Reviewer: Daniel Kurtz <djk...@chromium.org>
Gerrit-Reviewer: Derek Basehore <dbas...@chromium.org>
Gerrit-Reviewer: Dmitry Torokhov <dt...@chromium.org>
Gerrit-Reviewer: Dominik Behr <db...@chromium.org>
Gerrit-Reviewer: Doug Anderson <dian...@chromium.org>
Gerrit-Reviewer: Eddie Cai <eddi...@rock-chips.com>
Gerrit-Reviewer: Heiko Stübner <mmi...@googlemail.com>
Gerrit-Reviewer: Jeffy Chen <jeffy...@rock-chips.com>
Gerrit-Reviewer: Julius Werner <jwe...@chromium.org>
Gerrit-Reviewer: Kever Yang <kever...@rock-chips.com>
Gerrit-Reviewer: Shunqian Zheng <zhe...@rock-chips.com>
Gerrit-Reviewer: Sonny Rao <sonn...@chromium.org>
Gerrit-Reviewer: Tony Xie <tony...@rock-chips.com>
Gerrit-Reviewer: chrome-internal-fetch <chrome-int...@google.com>
Gerrit-Reviewer: 智情 姚 <mark...@rock-chips.com>
Gerrit-Reviewer: 王 晓腾 <caesa...@rock-chips.com>

Gerrit-HasComments: No

[chrome-internal-fetch (Gerrit)]

chrome-internal-fetch has submitted this change and it was merged.

Change subject: FROMLIST: ARM: rockchip: add suspend and resume for RK3288
......................................................................


FROMLIST: ARM: rockchip: add suspend and resume for RK3288

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

Conflicts:
arch/arm/mach-rockchip/Makefile
arch/arm/mach-rockchip/rockchip.c
...conflicts due to local l2 cache code (we skipped some upstream
cleanup commits) and due to power domain patches.

BUG=chrome-os-partner:32377
TEST=It can suspend by "echo mem > /sys/power/state", and resume by power
button

Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Signed-off-by: Tony Xie <x...@rock-chips.com>
Signed-off-by: Chris Zhong <z...@rock-chips.com>
Tested-by: Doug Anderson <dian...@chromium.org>
Reviewed-by: Doug Anderson <dian...@chromium.org>
Signed-off-by: Doug Anderson <dian...@chromium.org>
(am from https://patchwork.kernel.org/patch/5311641/)

Reviewed-on: https://chromium-review.googlesource.com/215780

---
M arch/arm/mach-rockchip/Makefile
A arch/arm/mach-rockchip/pm.c
A arch/arm/mach-rockchip/pm.h
M arch/arm/mach-rockchip/rockchip.c
A arch/arm/mach-rockchip/sleep.S
5 files changed, 439 insertions(+), 0 deletions(-)

Approvals:
Doug Anderson: Looks good to me, approved; Ready; Verified



diff --git a/arch/arm/mach-rockchip/Makefile
b/arch/arm/mach-rockchip/Makefile
index ca39efb..fa1ffbb 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.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..e059b84
--- /dev/null
+++ b/arch/arm/mach-rockchip/pm.c
@@ -0,0 +1,264 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony...@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"
+
+/* These enum are option of low power mode */
+enum {
+ ROCKCHIP_ARM_OFF_LOGIC_NORMAL = 0,
+ ROCKCHIP_ARM_OFF_LOGIC_DEEP = 1,
+};
+
+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 u32 rk3288_pmu_pwr_mode_con;
+static u32 rk3288_sgrf_soc_con0;
+
+static inline u32 rk3288_l2_config(void)
+{
+ u32 l2ctlr;
+
+ asm("mrc p15, 1, %0, c9, c0, 2" : "=r" (l2ctlr));
+ return l2ctlr;
+}
+
+static void rk3288_config_bootdata(void)
+{
+ 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();
+}
+
+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,
+ SGRF_FAST_BOOT_EN | SGRF_FAST_BOOT_EN_WRITE);
+
+ /* 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 | SGRF_FAST_BOOT_EN_WRITE);
+}
+
+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_slp_mode_set(ROCKCHIP_ARM_OFF_LOGIC_NORMAL);
+
+ cpu_suspend(0, rockchip_lpmode_enter);
+
+ rk3288_slp_mode_set_resume();
+
+ 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;
+
+ rk3288_config_bootdata();
+
+ /* 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..99722d0
--- /dev/null
+++ b/arch/arm/mach-rockchip/pm.h
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony...@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;
+
+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 BIT(8)
+#define SGRF_FAST_BOOT_EN_WRITE BIT(24)
+
+#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 c06bb16..be9ab79 100644
--- a/arch/arm/mach-rockchip/rockchip.c
+++ b/arch/arm/mach-rockchip/rockchip.c
@@ -23,10 +23,12 @@
#include <asm/mach/map.h>
#include <asm/hardware/cache-l2x0.h>
#include "core.h"
+#include "pm.h"

static void __init rockchip_dt_init(void)
{
l2x0_of_init(0, ~0UL);
+ rockchip_suspend_init();
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
platform_device_register_simple("cpufreq-cpu0", 0, NULL, 0);
}
diff --git a/arch/arm/mach-rockchip/sleep.S b/arch/arm/mach-rockchip/sleep.S
new file mode 100644
index 0000000..2eec9a3
--- /dev/null
+++ b/arch/arm/mach-rockchip/sleep.S
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony...@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_cpu_code
+ bx r1
+ENDPROC(rockchip_slp_cpu_resume)
+
+/* Parameters filled in by the kernel */
+
+/* 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

--
To view, visit https://chromium-review.googlesource.com/215780
To unsubscribe, visit https://chromium-review.googlesource.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If5b4dc6b97414deff9f6b4719f95268c949fdf17
Gerrit-PatchSet: 35