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

[PATCH v6 0/7] 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 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:
- reset-author
- use "__maybe_unused" annotation
- add pinctrl_force_default() in the error case
- modify comments
- use rk3288_bootram_sz for memcpy size
- fixed error of sram save and restore
- change the size of sram in example
- change size to 4k

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

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

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

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

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

-- 
1.7.9.5

[PATCH v6 5/7] 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 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.7.9.5

[PATCH v6 6/7] 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 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 874e66d..248c349 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -439,6 +439,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>;
@@ -634,6 +639,12 @@
 			bias-disable;
 		};
 
+		sleep {
+			global_pwroff: global-pwroff {
+				rockchip,pins = <0 0 RK_FUNC_1 &pcfg_pull_none>;
+			};
+		};
+
 		i2c0 {
 			i2c0_xfer: i2c0-xfer {
 				rockchip,pins = <0 15 RK_FUNC_1 &pcfg_pull_none>,
-- 
1.7.9.5

[PATCH v6 7/7] 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 v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

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

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

Re: [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808

[Doug Anderson]

Chris,

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

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

You should using the new patches from Chanwoo.

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

In your case, it would look like:

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

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

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

-Doug
--
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 v6 7/7] ARM: dts: add suspend voltage setting for RK808

[Javier Martinez Canillas]

Hello Chris,

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

Agreed.

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

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

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

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

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

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

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

Best regards,
Javier

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

Re: [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808

[Doug Anderson]

Javier,

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

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

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

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

-Doug
--
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 v6 0/7] This is the 1st version of suspend for RK3288.

[Kevin Hilman]

Hi Chris,

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

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

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

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

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

Thanks,

Kevin

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

[Heiko Stübner]

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

When testing this series it did go to sleep with

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

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


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

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


Heiko


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

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

[Doug Anderson]

Heiko,

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

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

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

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


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

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

Re: [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808

[Javier Martinez Canillas]

Hello Doug,

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

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

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

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

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

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

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

Best regards,
Javier

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