[PATCH v7 0/5] This is the 1st version of suspend for RK3288.

[PATCH v7 0/5] This is the 1st version of suspend for RK3288.

[Chris Zhong]

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

Changes in v7:
- get rid all of unused code
- add regulator-state-mem sub node for suspend

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

Changes in v5:
- modify comments
- use rk3288_bootram_sz for memcpy size
- fixed error of sram save and restore
- change the size of sram in example
- change size to 4k

Changes in v4:
- remove grf regmap

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

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

Chris Zhong (5):
  clk: rockchip: RK3288: add suspend and resume
  ARM: rockchip: add suspend and resume for RK3288
  ARM: rockchip: Add pmu-sram binding
  ARM: dts: add RK3288 suspend support
  ARM: dts: add suspend voltage setting for RK808

 .../devicetree/bindings/arm/rockchip/pmu-sram.txt  |  16 ++
 arch/arm/boot/dts/rk3288-evb-rk808.dts             |  46 +++-
 arch/arm/boot/dts/rk3288.dtsi                      |  11 +
 arch/arm/mach-rockchip/Makefile                    |   1 +
 arch/arm/mach-rockchip/pm.c                        | 264 +++++++++++++++++++++
 arch/arm/mach-rockchip/pm.h                        |  99 ++++++++
 arch/arm/mach-rockchip/rockchip.c                  |   2 +
 arch/arm/mach-rockchip/sleep.S                     |  73 ++++++
 drivers/clk/rockchip/clk-rk3288.c                  |  60 +++++
 9 files changed, 571 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt
 create mode 100644 arch/arm/mach-rockchip/pm.c
 create mode 100644 arch/arm/mach-rockchip/pm.h
 create mode 100644 arch/arm/mach-rockchip/sleep.S

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 3/5] ARM: rockchip: Add pmu-sram binding

[Chris Zhong]

The pmu-sram is used to store resume code, suspend/resume need get the
address of it. Therefore add a binding and documentation for it.

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

---

Changes in v7: None
Changes in v6:
- change pmu_intmem@ff720000 to sram@ff720000

Changes in v5:
- change the size of sram in example

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

 .../devicetree/bindings/arm/rockchip/pmu-sram.txt        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt

diff --git a/Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt b/Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt
new file mode 100644
index 0000000..6b42fda
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt
@@ -0,0 +1,16 @@
+Rockchip SRAM for pmu:
+------------------------------
+
+The sram of pmu is used to store the function of resume from maskrom(the 1st
+level loader). This is a common use of the "pmu-sram" because it keeps power
+even in low power states in the system.
+
+Required node properties:
+- compatible : should be "rockchip,rk3288-pmu-sram"
+- reg : physical base address and the size of the registers window
+
+Example:
+	sram@ff720000 {
+		compatible = "rockchip,rk3288-pmu-sram", "mmio-sram";
+		reg = <0xff720000 0x1000>;
+	};
-- 
1.9.1

[PATCH v7 4/5] ARM: dts: add RK3288 suspend support

[Chris Zhong]

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

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

---

Changes in v7: None
Changes in v6:
- change pmu_intmem@ff720000 to sram@ff720000

Changes in v5:
- change size to 4k

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

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

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index cfc4378..0747c30 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -462,6 +462,11 @@
 		status = "disabled";
 	};
 
+	sram@ff720000 {
+		compatible = "rockchip,rk3288-pmu-sram", "mmio-sram";
+		reg = <0xff720000 0x1000>;
+	};
+
 	pmu: power-management@ff730000 {
 		compatible = "rockchip,rk3288-pmu", "syscon";
 		reg = <0xff730000 0x100>;
@@ -667,6 +672,12 @@
 			bias-disable;
 		};
 
+		sleep {
+			global_pwroff: global-pwroff {
+				rockchip,pins = <0 0 RK_FUNC_1 &pcfg_pull_none>;
+			};
+		};
+
 		i2c0 {
 			i2c0_xfer: i2c0-xfer {
 				rockchip,pins = <0 15 RK_FUNC_1 &pcfg_pull_none>,
-- 
1.9.1

[PATCH v7 5/5] ARM: dts: add suspend voltage setting for RK808

[Chris Zhong]

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

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

---

Changes in v7:
- add regulator-state-mem sub node for suspend

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

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

diff --git a/arch/arm/boot/dts/rk3288-evb-rk808.dts b/arch/arm/boot/dts/rk3288-evb-rk808.dts
index d8c775e6..6145cf4 100644
--- a/arch/arm/boot/dts/rk3288-evb-rk808.dts
+++ b/arch/arm/boot/dts/rk3288-evb-rk808.dts
@@ -31,7 +31,7 @@
 		interrupt-parent = <&gpio0>;
 		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&pmic_int>;
+		pinctrl-0 = <&pmic_int &global_pwroff>;
 		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
@@ -50,6 +50,9 @@
 				regulator-min-microvolt = <750000>;
 				regulator-max-microvolt = <1350000>;
 				regulator-name = "vdd_arm";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vdd_gpu: DCDC_REG2 {
@@ -58,12 +61,19 @@
 				regulator-min-microvolt = <850000>;
 				regulator-max-microvolt = <1250000>;
 				regulator-name = "vdd_gpu";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vcc_ddr: DCDC_REG3 {
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-name = "vcc_ddr";
+				regulator-suspend-mem-enabled;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			vcc_io: DCDC_REG4 {
@@ -72,6 +82,9 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vcc_io";
+				regulator-state-mem {
+					regulator-suspend-microvolt = <3300000>;
+				};
 			};
 
 			vccio_pmu: LDO_REG1 {
@@ -80,6 +93,9 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vccio_pmu";
+				regulator-state-mem {
+					regulator-suspend-microvolt = <3300000>;
+				};
 			};
 
 			vcc_tp: LDO_REG2 {
@@ -88,6 +104,9 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vcc_tp";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vdd_10: LDO_REG3 {
@@ -96,6 +115,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-name = "vdd_10";
+				regulator-state-mem {
+					regulator-suspend-microvolt = <1000000>;
+				};
 			};
 
 			vcc18_lcd: LDO_REG4 {
@@ -104,6 +126,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-name = "vcc18_lcd";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vccio_sd: LDO_REG5 {
@@ -112,6 +137,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vccio_sd";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vdd10_lcd: LDO_REG6 {
@@ -120,6 +148,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-name = "vdd10_lcd";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vcc_18: LDO_REG7 {
@@ -128,6 +159,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-name = "vcc_18";
+				regulator-suspend-mem-microvolt = <1800000>;
+				regulator-state-mem {
+					regulator-suspend-microvolt = <1800000>;
+				};
 			};
 
 			vcca_codec: LDO_REG8 {
@@ -136,18 +171,27 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vcca_codec";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vcc_wl: SWITCH_REG1 {
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-name = "vcc_wl";
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			vcc_lcd: SWITCH_REG2 {
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-name = "vcc_lcd";
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 		};
 	};
-- 
1.9.1

Re: [PATCH v7 0/5] This is the 1st version of suspend for RK3288.

[Kevin Hilman]

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

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

The cover letter still doesn't state what this series applies to, or
what its dependencies are for testing, even though it was requested in
earlier reviews[1].  I discovered (again) by trial and error it applies
to current linux-next.  I also discovered (as was earlier discussed[2])
that it still does not resume using current upstream code, and those
dependencies are not described here either.  These are the kinds of
things that are crucial in a cover letter in order to help reviewers and
testers not have to spend time digging through the archives trying to
remember from the previous round of reviews. 

Please, please list the out-of-tree dependencies, and how to test,
including how you tested it, and on what hardware.

Speaking of earlier reviews, I've noticed that after reviewing this
series multiple times, you never respond to questions and/or comments.
Two-way communication is important when collaborating on getting complex
code this upstream, so please take some time to acknowledge the comments
of reviewers and engage in discussion when questions are asked.  Even if
it's as simple as "OK, I'll fix it in the next version", that helps
reviewers know that they're not wasting their time.

Kevin

[1] https://lkml.org/lkml/2014/10/29/759
[2] https://lkml.org/lkml/2014/10/29/881

Re: [PATCH v7 4/5] ARM: dts: add RK3288 suspend support

[Doug Anderson]

Chris,

On Fri, Nov 7, 2014 at 5:49 AM, Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> add pmu sram node for suspend, add global_pwroff pinctrl.
> The pmu sram is used to store the resume code.
> global_pwroff is held low level at work, it would be pull to high
> when entering suspend. reference this in the board DTS file since
> some boards need it.
>
> Signed-off-by: Tony Xie <xxx-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Reviewed-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Tested-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>
> ---
>
> Changes in v7: None
> Changes in v6:
> - change pmu_intmem@ff720000 to sram@ff720000
>
> Changes in v5:
> - change size to 4k
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - put "rockchip,rk3288-pmu-sram" to first
>
>  arch/arm/boot/dts/rk3288.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)

I was sorta hoping that when you sent out v7 you'd incorporate the
extra dts bits that you sent me and that I had stashed at
<https://chromium-review.googlesource.com/#/c/226826/>.  Maybe you
could do it if you send out a v8.

If you don't send out a v8 we could always land those bits separately.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 0/5] This is the 1st version of suspend for RK3288.

[Heiko Stübner]

Hi Chris,

Am Freitag, 7. November 2014, 14:48:20 schrieb Kevin Hilman:
> Chris Zhong <zyw@rock-chips.com> writes:
> > RK3288 can shut down the cpu, gpu and other device controllers in suspend,
> > and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
> > process of suspend, pull the pin to low again when resume.
> 
> The cover letter still doesn't state what this series applies to, or
> what its dependencies are for testing, even though it was requested in
> earlier reviews[1].  I discovered (again) by trial and error it applies
> to current linux-next.  I also discovered (as was earlier discussed[2])
> that it still does not resume using current upstream code, and those
> dependencies are not described here either.  These are the kinds of
> things that are crucial in a cover letter in order to help reviewers and
> testers not have to spend time digging through the archives trying to
> remember from the previous round of reviews.
> 
> Please, please list the out-of-tree dependencies, and how to test,
> including how you tested it, and on what hardware.

I'll second what Kevin said.

I guess the regulator suspend handling [0] is one of the requirements, but I'd 
think there is more. And while Doug had a quite long list of suspend-related 
patches in his try, for now we'll need the minimal set to enable this series 
to sucessfully wake the system again after going to suspend.


Heiko


[0] 
https://git.kernel.org/cgit/linux/kernel/git/broonie/regulator.git/log/?h=topic/suspend

Re: [PATCH v7 0/5] This is the 1st version of suspend for RK3288.

[Doug Anderson]

Hi,

On Mon, Nov 10, 2014 at 4:40 PM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> Hi Chris,
>
> Am Freitag, 7. November 2014, 14:48:20 schrieb Kevin Hilman:
>> Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org> writes:
>> > RK3288 can shut down the cpu, gpu and other device controllers in suspend,
>> > and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
>> > process of suspend, pull the pin to low again when resume.
>>
>> The cover letter still doesn't state what this series applies to, or
>> what its dependencies are for testing, even though it was requested in
>> earlier reviews[1].  I discovered (again) by trial and error it applies
>> to current linux-next.  I also discovered (as was earlier discussed[2])
>> that it still does not resume using current upstream code, and those
>> dependencies are not described here either.  These are the kinds of
>> things that are crucial in a cover letter in order to help reviewers and
>> testers not have to spend time digging through the archives trying to
>> remember from the previous round of reviews.
>>
>> Please, please list the out-of-tree dependencies, and how to test,
>> including how you tested it, and on what hardware.
>
> I'll second what Kevin said.
>
> I guess the regulator suspend handling [0] is one of the requirements, but I'd
> think there is more. And while Doug had a quite long list of suspend-related
> patches in his try, for now we'll need the minimal set to enable this series
> to sucessfully wake the system again after going to suspend.

With the current series we have in the Chrome OS tree, the WIP power
domain patches are a requirement, which makes testing very hard.  I've
suggested that Chris try going back to leaving the GPU on his patches.
It's possible you could get basic suspend/resume without power domain
patches in that case.  Once power domain stuff is in good shape then
we can turn off the GPU, I think.

...all of those things are needed to actually get the system into the
lowest power state, but starting simple makes a lot of sense.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 0/5] This is the 1st version of suspend for RK3288.

[Chris Zhong]

Hi Kevin

On 11/08/2014 06:48 AM, Kevin Hilman wrote:
> Chris Zhong <zyw@rock-chips.com> writes:
>
>> RK3288 can shut down the cpu, gpu and other device controllers in suspend,
>> and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
>> process of suspend, pull the pin to low again when resume.
> The cover letter still doesn't state what this series applies to, or
> what its dependencies are for testing, even though it was requested in
> earlier reviews[1].  I discovered (again) by trial and error it applies
> to current linux-next.  I also discovered (as was earlier discussed[2])
> that it still does not resume using current upstream code, and those
> dependencies are not described here either.  These are the kinds of
> things that are crucial in a cover letter in order to help reviewers and
> testers not have to spend time digging through the archives trying to
> remember from the previous round of reviews.
Thank you for your review, I will perfect the cover letter in next 
version patches.

> Please, please list the out-of-tree dependencies, and how to test,
> including how you tested it, and on what hardware.
>
> Speaking of earlier reviews, I've noticed that after reviewing this
> series multiple times, you never respond to questions and/or comments.
> Two-way communication is important when collaborating on getting complex
> code this upstream, so please take some time to acknowledge the comments
> of reviewers and engage in discussion when questions are asked.  Even if
> it's as simple as "OK, I'll fix it in the next version", that helps
> reviewers know that they're not wasting their time.
OK, I'm going to respond all, even though only "Done".

> Kevin
>
> [1] https://lkml.org/lkml/2014/10/29/759
> [2] https://lkml.org/lkml/2014/10/29/881
>
>
>