[PATCH 0/3] powerpc/83xx: Sleep and deep sleep support for MPC8315E-RDB

[PATCH 0/3] powerpc/83xx: Sleep and deep sleep support for MPC8315E-RDB

[Anton Vorontsov]

Hi all,

Here are some patches to make suspend/resume work on MPC8315E-RDB...

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Anton Vorontsov]

Currently 83xx PMC driver clears deep_sleeping variable very early,
before devices are resumed. This makes fsl_deep_sleep() unusable in
drivers' resume() callback.

Sure, drivers can store fsl_deep_sleep() value on suspend and use
the stored value on resume. But a better solution is to postpone
clearing the deep_sleeping variable, i.e. move it into finish()
callback.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/83xx/suspend.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index d306f07..b0c2619 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -194,7 +194,7 @@ out:
 	return ret;
 }
 
-static void mpc83xx_suspend_finish(void)
+static void mpc83xx_suspend_end(void)
 {
 	deep_sleeping = 0;
 }
@@ -278,7 +278,7 @@ static struct platform_suspend_ops mpc83xx_suspend_ops = {
 	.valid = mpc83xx_suspend_valid,
 	.begin = mpc83xx_suspend_begin,
 	.enter = mpc83xx_suspend_enter,
-	.finish = mpc83xx_suspend_finish,
+	.end = mpc83xx_suspend_end,
 };
 
 static int pmc_probe(struct of_device *ofdev,
-- 
1.6.3.3

[PATCH 2/3] powerpc/83xx/suspend: Save and restore SICRL, SICRH and SCCR

[Anton Vorontsov]

We need to save SICRL, SICRH and SCCR registers on suspend, and restore
them on resume. Otherwise, we lose IO and clocks setup on MPC8315E-RDB
boards when ULPI USB PHY is used (non-POR setup).

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/83xx/suspend.c |   48 +++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index b0c2619..4380534 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -32,6 +32,7 @@
 #define PMCCR1_NEXT_STATE       0x0C /* Next state for power management */
 #define PMCCR1_NEXT_STATE_SHIFT 2
 #define PMCCR1_CURR_STATE       0x03 /* Current state for power management*/
+#define IMMR_SYSCR_OFFSET       0x100
 #define IMMR_RCW_OFFSET         0x900
 #define RCW_PCI_HOST            0x80000000
 
@@ -78,6 +79,22 @@ struct mpc83xx_clock {
 	u32 sccr;
 };
 
+struct mpc83xx_syscr {
+	__be32 sgprl;
+	__be32 sgprh;
+	__be32 spridr;
+	__be32 :32;
+	__be32 spcr;
+	__be32 sicrl;
+	__be32 sicrh;
+};
+
+struct mpc83xx_saved {
+	u32 sicrl;
+	u32 sicrh;
+	u32 sccr;
+};
+
 struct pmc_type {
 	int has_deep_sleep;
 };
@@ -87,6 +104,8 @@ static int has_deep_sleep, deep_sleeping;
 static int pmc_irq;
 static struct mpc83xx_pmc __iomem *pmc_regs;
 static struct mpc83xx_clock __iomem *clock_regs;
+static struct mpc83xx_syscr __iomem *syscr_regs;
+static struct mpc83xx_saved saved_regs;
 static int is_pci_agent, wake_from_pci;
 static phys_addr_t immrbase;
 static int pci_pm_state;
@@ -137,6 +156,20 @@ static irqreturn_t pmc_irq_handler(int irq, void *dev_id)
 	return ret;
 }
 
+static void mpc83xx_suspend_restore_regs(void)
+{
+	out_be32(&syscr_regs->sicrl, saved_regs.sicrl);
+	out_be32(&syscr_regs->sicrh, saved_regs.sicrh);
+	out_be32(&clock_regs->sccr, saved_regs.sccr);
+}
+
+static void mpc83xx_suspend_save_regs(void)
+{
+	saved_regs.sicrl = in_be32(&syscr_regs->sicrl);
+	saved_regs.sicrh = in_be32(&syscr_regs->sicrh);
+	saved_regs.sccr = in_be32(&clock_regs->sccr);
+}
+
 static int mpc83xx_suspend_enter(suspend_state_t state)
 {
 	int ret = -EAGAIN;
@@ -166,6 +199,8 @@ static int mpc83xx_suspend_enter(suspend_state_t state)
 	 */
 
 	if (deep_sleeping) {
+		mpc83xx_suspend_save_regs();
+
 		out_be32(&pmc_regs->mask, PMCER_ALL);
 
 		out_be32(&pmc_regs->config1,
@@ -179,6 +214,8 @@ static int mpc83xx_suspend_enter(suspend_state_t state)
 		         in_be32(&pmc_regs->config1) & ~PMCCR1_POWER_OFF);
 
 		out_be32(&pmc_regs->mask, PMCER_PMCI);
+
+		mpc83xx_suspend_restore_regs();
 	} else {
 		out_be32(&pmc_regs->mask, PMCER_PMCI);
 
@@ -333,12 +370,23 @@ static int pmc_probe(struct of_device *ofdev,
 		goto out_pmc;
 	}
 
+	if (has_deep_sleep) {
+		syscr_regs = ioremap(immrbase + IMMR_SYSCR_OFFSET,
+				     sizeof(*syscr_regs));
+		if (!syscr_regs) {
+			ret = -ENOMEM;
+			goto out_syscr;
+		}
+	}
+
 	if (is_pci_agent)
 		mpc83xx_set_agent();
 
 	suspend_set_ops(&mpc83xx_suspend_ops);
 	return 0;
 
+out_syscr:
+	iounmap(clock_regs);
 out_pmc:
 	iounmap(pmc_regs);
 out:
-- 
1.6.3.3

[PATCH 3/3] powerpc/83xx: Add power management support for MPC8315E-RDB boards

[Anton Vorontsov]

- Add nodes for PMC and GTM controllers. GTM4 can be used as a wakeup
  source;

- Add fsl,magic-packet properties to eTSEC nodes, i.e. wake-on-lan
  support. Unlike MPC8313 processors, MPC8315 can resume from deep
  sleep upon magic packet reception;

- Add proper sleep = <> properties;

- DMA and PCI share a single clock soruce, so put them into "sleep
  nexus" node (the same we do for MPC8313E-RDB boards);

- I2C and Encryption core also share a single clock, so do the same:
  put i2c and crypto nodes into sleep-nexus.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc8315erdb.dts |  264 ++++++++++++++++++++-------------
 1 files changed, 161 insertions(+), 103 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8315erdb.dts b/arch/powerpc/boot/dts/mpc8315erdb.dts
index 32e10f5..406ebc3 100644
--- a/arch/powerpc/boot/dts/mpc8315erdb.dts
+++ b/arch/powerpc/boot/dts/mpc8315erdb.dts
@@ -109,26 +109,47 @@
 			reg = <0x200 0x100>;
 		};
 
-		i2c@3000 {
+		sleep-nexus {
 			#address-cells = <1>;
-			#size-cells = <0>;
-			cell-index = <0>;
-			compatible = "fsl-i2c";
-			reg = <0x3000 0x100>;
-			interrupts = <14 0x8>;
-			interrupt-parent = <&ipic>;
-			dfsrr;
-			rtc@68 {
-				compatible = "dallas,ds1339";
-				reg = <0x68>;
+			#size-cells = <1>;
+			compatible = "simple-bus";
+			sleep = <&pmc 0x03000000>;
+			ranges;
+
+			i2c@3000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				cell-index = <0>;
+				compatible = "fsl-i2c";
+				reg = <0x3000 0x100>;
+				interrupts = <14 0x8>;
+				interrupt-parent = <&ipic>;
+				dfsrr;
+				rtc@68 {
+					compatible = "dallas,ds1339";
+					reg = <0x68>;
+				};
+
+				mcu_pio: mcu@a {
+					#gpio-cells = <2>;
+					compatible = "fsl,mc9s08qg8-mpc8315erdb",
+						     "fsl,mcu-mpc8349emitx";
+					reg = <0x0a>;
+					gpio-controller;
+				};
 			};
 
-			mcu_pio: mcu@a {
-				#gpio-cells = <2>;
-				compatible = "fsl,mc9s08qg8-mpc8315erdb",
-					     "fsl,mcu-mpc8349emitx";
-				reg = <0x0a>;
-				gpio-controller;
+			crypto@30000 {
+				compatible = "fsl,sec3.3", "fsl,sec3.1", "fsl,sec3.0",
+					     "fsl,sec2.4", "fsl,sec2.2", "fsl,sec2.1",
+					     "fsl,sec2.0";
+				reg = <0x30000 0x10000>;
+				interrupts = <11 0x8>;
+				interrupt-parent = <&ipic>;
+				fsl,num-channels = <4>;
+				fsl,channel-fifo-len = <24>;
+				fsl,exec-units-mask = <0x97c>;
+				fsl,descriptor-types-mask = <0x3ab0abf>;
 			};
 		};
 
@@ -141,45 +162,6 @@
 			mode = "cpu";
 		};
 
-		dma@82a8 {
-			#address-cells = <1>;
-			#size-cells = <1>;
-			compatible = "fsl,mpc8315-dma", "fsl,elo-dma";
-			reg = <0x82a8 4>;
-			ranges = <0 0x8100 0x1a8>;
-			interrupt-parent = <&ipic>;
-			interrupts = <71 8>;
-			cell-index = <0>;
-			dma-channel@0 {
-				compatible = "fsl,mpc8315-dma-channel", "fsl,elo-dma-channel";
-				reg = <0 0x80>;
-				cell-index = <0>;
-				interrupt-parent = <&ipic>;
-				interrupts = <71 8>;
-			};
-			dma-channel@80 {
-				compatible = "fsl,mpc8315-dma-channel", "fsl,elo-dma-channel";
-				reg = <0x80 0x80>;
-				cell-index = <1>;
-				interrupt-parent = <&ipic>;
-				interrupts = <71 8>;
-			};
-			dma-channel@100 {
-				compatible = "fsl,mpc8315-dma-channel", "fsl,elo-dma-channel";
-				reg = <0x100 0x80>;
-				cell-index = <2>;
-				interrupt-parent = <&ipic>;
-				interrupts = <71 8>;
-			};
-			dma-channel@180 {
-				compatible = "fsl,mpc8315-dma-channel", "fsl,elo-dma-channel";
-				reg = <0x180 0x28>;
-				cell-index = <3>;
-				interrupt-parent = <&ipic>;
-				interrupts = <71 8>;
-			};
-		};
-
 		usb@23000 {
 			compatible = "fsl-usb2-dr";
 			reg = <0x23000 0x1000>;
@@ -188,6 +170,7 @@
 			interrupt-parent = <&ipic>;
 			interrupts = <38 0x8>;
 			phy_type = "utmi";
+			sleep = <&pmc 0x00c00000>;
 		};
 
 		enet0: ethernet@24000 {
@@ -204,6 +187,8 @@
 			interrupt-parent = <&ipic>;
 			tbi-handle = <&tbi0>;
 			phy-handle = < &phy0 >;
+			sleep = <&pmc 0xc0000000>;
+			fsl,magic-packet;
 
 			mdio@520 {
 				#address-cells = <1>;
@@ -246,6 +231,8 @@
 			interrupt-parent = <&ipic>;
 			tbi-handle = <&tbi1>;
 			phy-handle = < &phy1 >;
+			sleep = <&pmc 0x30000000>;
+			fsl,magic-packet;
 
 			mdio@520 {
 				#address-cells = <1>;
@@ -280,25 +267,13 @@
 			interrupt-parent = <&ipic>;
 		};
 
-		crypto@30000 {
-			compatible = "fsl,sec3.3", "fsl,sec3.1", "fsl,sec3.0",
-				     "fsl,sec2.4", "fsl,sec2.2", "fsl,sec2.1",
-				     "fsl,sec2.0";
-			reg = <0x30000 0x10000>;
-			interrupts = <11 0x8>;
-			interrupt-parent = <&ipic>;
-			fsl,num-channels = <4>;
-			fsl,channel-fifo-len = <24>;
-			fsl,exec-units-mask = <0x97c>;
-			fsl,descriptor-types-mask = <0x3ab0abf>;
-		};
-
 		sata@18000 {
 			compatible = "fsl,mpc8315-sata", "fsl,pq-sata";
 			reg = <0x18000 0x1000>;
 			cell-index = <1>;
 			interrupts = <44 0x8>;
 			interrupt-parent = <&ipic>;
+			sleep = <&pmc 0x00003000>;
 		};
 
 		sata@19000 {
@@ -307,6 +282,23 @@
 			cell-index = <2>;
 			interrupts = <45 0x8>;
 			interrupt-parent = <&ipic>;
+			sleep = <&pmc 0x00000c00>;
+		};
+
+		gtm1: timer@500 {
+			compatible = "fsl,mpc8315-gtm", "fsl,gtm";
+			reg = <0x500 0x100>;
+			interrupts = <90 8 78 8 84 8 72 8>;
+			interrupt-parent = <&ipic>;
+			clock-frequency = <133333333>;
+		};
+
+		timer@600 {
+			compatible = "fsl,mpc8315-gtm", "fsl,gtm";
+			reg = <0x600 0x100>;
+			interrupts = <91 8 79 8 85 8 73 8>;
+			interrupt-parent = <&ipic>;
+			clock-frequency = <133333333>;
 		};
 
 		/* IPIC
@@ -337,42 +329,106 @@
 				      0x59 0x8>;
 			interrupt-parent = < &ipic >;
 		};
+
+		pmc: power@b00 {
+			compatible = "fsl,mpc8315-pmc", "fsl,mpc8313-pmc",
+				     "fsl,mpc8349-pmc";
+			reg = <0xb00 0x100 0xa00 0x100>;
+			interrupts = <80 8>;
+			interrupt-parent = <&ipic>;
+			fsl,mpc8313-wakeup-timer = <&gtm1>;
+		};
 	};
 
-	pci0: pci@e0008500 {
-		interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
-		interrupt-map = <
-				/* IDSEL 0x0E -mini PCI */
-				 0x7000 0x0 0x0 0x1 &ipic 18 0x8
-				 0x7000 0x0 0x0 0x2 &ipic 18 0x8
-				 0x7000 0x0 0x0 0x3 &ipic 18 0x8
-				 0x7000 0x0 0x0 0x4 &ipic 18 0x8
-
-				/* IDSEL 0x0F -mini PCI */
-				 0x7800 0x0 0x0 0x1 &ipic 17 0x8
-				 0x7800 0x0 0x0 0x2 &ipic 17 0x8
-				 0x7800 0x0 0x0 0x3 &ipic 17 0x8
-				 0x7800 0x0 0x0 0x4 &ipic 17 0x8
-
-				/* IDSEL 0x10 - PCI slot */
-				 0x8000 0x0 0x0 0x1 &ipic 48 0x8
-				 0x8000 0x0 0x0 0x2 &ipic 17 0x8
-				 0x8000 0x0 0x0 0x3 &ipic 48 0x8
-				 0x8000 0x0 0x0 0x4 &ipic 17 0x8>;
-		interrupt-parent = <&ipic>;
-		interrupts = <66 0x8>;
-		bus-range = <0x0 0x0>;
-		ranges = <0x02000000 0 0x90000000 0x90000000 0 0x10000000
-			  0x42000000 0 0x80000000 0x80000000 0 0x10000000
-			  0x01000000 0 0x00000000 0xe0300000 0 0x00100000>;
-		clock-frequency = <66666666>;
-		#interrupt-cells = <1>;
-		#size-cells = <2>;
-		#address-cells = <3>;
-		reg = <0xe0008500 0x100		/* internal registers */
-		       0xe0008300 0x8>;		/* config space access registers */
-		compatible = "fsl,mpc8349-pci";
-		device_type = "pci";
+	sleep-nexus {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		sleep = <&pmc 0x00010000>;
+		ranges;
+
+		pci0: pci@e0008500 {
+			interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
+			interrupt-map = <
+					/* IDSEL 0x0E -mini PCI */
+					 0x7000 0x0 0x0 0x1 &ipic 18 0x8
+					 0x7000 0x0 0x0 0x2 &ipic 18 0x8
+					 0x7000 0x0 0x0 0x3 &ipic 18 0x8
+					 0x7000 0x0 0x0 0x4 &ipic 18 0x8
+
+					/* IDSEL 0x0F -mini PCI */
+					 0x7800 0x0 0x0 0x1 &ipic 17 0x8
+					 0x7800 0x0 0x0 0x2 &ipic 17 0x8
+					 0x7800 0x0 0x0 0x3 &ipic 17 0x8
+					 0x7800 0x0 0x0 0x4 &ipic 17 0x8
+
+					/* IDSEL 0x10 - PCI slot */
+					 0x8000 0x0 0x0 0x1 &ipic 48 0x8
+					 0x8000 0x0 0x0 0x2 &ipic 17 0x8
+					 0x8000 0x0 0x0 0x3 &ipic 48 0x8
+					 0x8000 0x0 0x0 0x4 &ipic 17 0x8>;
+			interrupt-parent = <&ipic>;
+			interrupts = <66 0x8>;
+			bus-range = <0x0 0x0>;
+			ranges = <0x02000000 0 0x90000000 0x90000000 0 0x10000000
+				  0x42000000 0 0x80000000 0x80000000 0 0x10000000
+				  0x01000000 0 0x00000000 0xe0300000 0 0x00100000>;
+			clock-frequency = <66666666>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			#address-cells = <3>;
+			reg = <0xe0008500 0x100	/* internal registers */
+			       0xe0008300 0x8>;	/* config space access registers */
+			compatible = "fsl,mpc8349-pci";
+			device_type = "pci";
+		};
+
+		dma@e00082a8 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "fsl,mpc8315-dma", "fsl,elo-dma";
+			reg = <0xe00082a8 4>;
+			ranges = <0 0xe0008100 0x1a8>;
+			interrupt-parent = <&ipic>;
+			interrupts = <71 8>;
+			cell-index = <0>;
+
+			dma-channel@0 {
+				compatible = "fsl,mpc8315-dma-channel",
+					     "fsl,elo-dma-channel";
+				reg = <0 0x80>;
+				cell-index = <0>;
+				interrupt-parent = <&ipic>;
+				interrupts = <71 8>;
+			};
+
+			dma-channel@80 {
+				compatible = "fsl,mpc8315-dma-channel",
+					     "fsl,elo-dma-channel";
+				reg = <0x80 0x80>;
+				cell-index = <1>;
+				interrupt-parent = <&ipic>;
+				interrupts = <71 8>;
+			};
+
+			dma-channel@100 {
+				compatible = "fsl,mpc8315-dma-channel",
+					     "fsl,elo-dma-channel";
+				reg = <0x100 0x80>;
+				cell-index = <2>;
+				interrupt-parent = <&ipic>;
+				interrupts = <71 8>;
+			};
+
+			dma-channel@180 {
+				compatible = "fsl,mpc8315-dma-channel",
+					     "fsl,elo-dma-channel";
+				reg = <0x180 0x28>;
+				cell-index = <3>;
+				interrupt-parent = <&ipic>;
+				interrupts = <71 8>;
+			};
+		};
 	};
 
 	pci1: pcie@e0009000 {
@@ -390,6 +446,7 @@
 				 0 0 0 2 &ipic 1 8
 				 0 0 0 3 &ipic 1 8
 				 0 0 0 4 &ipic 1 8>;
+		sleep = <&pmc 0x00300000>;
 		clock-frequency = <0>;
 
 		pcie@0 {
@@ -421,6 +478,7 @@
 				 0 0 0 2 &ipic 2 8
 				 0 0 0 3 &ipic 2 8
 				 0 0 0 4 &ipic 2 8>;
+		sleep = <&pmc 0x000c0000>;
 		clock-frequency = <0>;
 
 		pcie@0 {
-- 
1.6.3.3

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Kumar Gala]

On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote:

> Currently 83xx PMC driver clears deep_sleeping variable very early,
> before devices are resumed. This makes fsl_deep_sleep() unusable in
> drivers' resume() callback.
>
> Sure, drivers can store fsl_deep_sleep() value on suspend and use
> the stored value on resume. But a better solution is to postpone
> clearing the deep_sleeping variable, i.e. move it into finish()
> callback.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/platforms/83xx/suspend.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)

Scott, any comments or an ack?

- k

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Scott Wood]

Kumar Gala wrote:
> 
> On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote:
> 
>> Currently 83xx PMC driver clears deep_sleeping variable very early,
>> before devices are resumed. This makes fsl_deep_sleep() unusable in
>> drivers' resume() callback.
>>
>> Sure, drivers can store fsl_deep_sleep() value on suspend and use
>> the stored value on resume. But a better solution is to postpone
>> clearing the deep_sleeping variable, i.e. move it into finish()
>> callback.
>>
>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>> ---
>> arch/powerpc/platforms/83xx/suspend.c |    4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> Scott, any comments or an ack?

ACK

-Scott

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Kumar Gala]

On Nov 5, 2009, at 10:57 AM, Scott Wood wrote:

> Kumar Gala wrote:
>> On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote:
>>> Currently 83xx PMC driver clears deep_sleeping variable very early,
>>> before devices are resumed. This makes fsl_deep_sleep() unusable in
>>> drivers' resume() callback.
>>>
>>> Sure, drivers can store fsl_deep_sleep() value on suspend and use
>>> the stored value on resume. But a better solution is to postpone
>>> clearing the deep_sleeping variable, i.e. move it into finish()
>>> callback.
>>>
>>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>>> ---
>>> arch/powerpc/platforms/83xx/suspend.c |    4 ++--
>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>> Scott, any comments or an ack?
>
> ACK

thanks, is that an ACK for all 3?

- k

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Scott Wood]

Kumar Gala wrote:
> 
> On Nov 5, 2009, at 10:57 AM, Scott Wood wrote:
> 
>> Kumar Gala wrote:
>>> On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote:
>>>> Currently 83xx PMC driver clears deep_sleeping variable very early,
>>>> before devices are resumed. This makes fsl_deep_sleep() unusable in
>>>> drivers' resume() callback.
>>>>
>>>> Sure, drivers can store fsl_deep_sleep() value on suspend and use
>>>> the stored value on resume. But a better solution is to postpone
>>>> clearing the deep_sleeping variable, i.e. move it into finish()
>>>> callback.
>>>>
>>>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>>>> ---
>>>> arch/powerpc/platforms/83xx/suspend.c |    4 ++--
>>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>> Scott, any comments or an ack?
>>
>> ACK
> 
> thanks, is that an ACK for all 3?

The first.  Patch 2 looks OK as well.

As for patch 3, Ben objected to the sleep-nexus stuff on IRC.

-Scott

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Kumar Gala]

On Nov 5, 2009, at 1:48 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> On Nov 5, 2009, at 10:57 AM, Scott Wood wrote:
>>> Kumar Gala wrote:
>>>> On Sep 23, 2009, at 2:01 PM, Anton Vorontsov wrote:
>>>>> Currently 83xx PMC driver clears deep_sleeping variable very  
>>>>> early,
>>>>> before devices are resumed. This makes fsl_deep_sleep() unusable  
>>>>> in
>>>>> drivers' resume() callback.
>>>>>
>>>>> Sure, drivers can store fsl_deep_sleep() value on suspend and use
>>>>> the stored value on resume. But a better solution is to postpone
>>>>> clearing the deep_sleeping variable, i.e. move it into finish()
>>>>> callback.
>>>>>
>>>>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>>>>> ---
>>>>> arch/powerpc/platforms/83xx/suspend.c |    4 ++--
>>>>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>>> Scott, any comments or an ack?
>>>
>>> ACK
>> thanks, is that an ACK for all 3?
>
> The first.  Patch 2 looks OK as well.
>
> As for patch 3, Ben objected to the sleep-nexus stuff on IRC.

Is sleep-nexus new?  I thought we've had that for a bit.

- k

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Scott Wood]

Kumar Gala wrote:
> On Nov 5, 2009, at 1:48 PM, Scott Wood wrote:
>> As for patch 3, Ben objected to the sleep-nexus stuff on IRC.
> 
> Is sleep-nexus new?  I thought we've had that for a bit.

It's been around in a few dts files, but as was noted, nothing uses this 
stuff yet.

-Scott

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Anton Vorontsov]

On Thu, Nov 05, 2009 at 02:03:14PM -0600, Scott Wood wrote:
> Kumar Gala wrote:
> >On Nov 5, 2009, at 1:48 PM, Scott Wood wrote:
> >>As for patch 3, Ben objected to the sleep-nexus stuff on IRC.
> >
> >Is sleep-nexus new?  I thought we've had that for a bit.
> 
> It's been around in a few dts files, but as was noted, nothing uses
> this stuff yet.

So I should just drop the sleep-nexus changes? I can also
prepare a patch that removes sleep-nexus from 8313rdb.dts.
But how should we handle the sleep = <> properties then?

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Scott Wood]

Anton Vorontsov wrote:
> On Thu, Nov 05, 2009 at 02:03:14PM -0600, Scott Wood wrote:
>> Kumar Gala wrote:
>>> On Nov 5, 2009, at 1:48 PM, Scott Wood wrote:
>>>> As for patch 3, Ben objected to the sleep-nexus stuff on IRC.
>>> Is sleep-nexus new?  I thought we've had that for a bit.
>> It's been around in a few dts files, but as was noted, nothing uses
>> this stuff yet.
> 
> So I should just drop the sleep-nexus changes? I can also
> prepare a patch that removes sleep-nexus from 8313rdb.dts.
> But how should we handle the sleep = <> properties then?

We could still have some sort of nexus node that is off to the side 
(pointed to with sleep-parent) and not inserting itself into the hierarchy.

Or, we could allow multiple nodes to refer to the same sleep ID, and use 
the ID rather than a node to tie things together.  If we do that, we'll 
probably want a simple index rather than a set of bits in the sleep 
property, so it can correspond to some kernel object that has some 
bookeeping info.  Perhaps we could tie into the clock bindings that were 
discussed on devtree-discuss in August.

-Scott

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Anton Vorontsov]

On Thu, Nov 05, 2009 at 02:25:15PM -0600, Scott Wood wrote:
> Anton Vorontsov wrote:
> >On Thu, Nov 05, 2009 at 02:03:14PM -0600, Scott Wood wrote:
> >>Kumar Gala wrote:
> >>>On Nov 5, 2009, at 1:48 PM, Scott Wood wrote:
> >>>>As for patch 3, Ben objected to the sleep-nexus stuff on IRC.
> >>>Is sleep-nexus new?  I thought we've had that for a bit.
> >>It's been around in a few dts files, but as was noted, nothing uses
> >>this stuff yet.
> >
> >So I should just drop the sleep-nexus changes? I can also
> >prepare a patch that removes sleep-nexus from 8313rdb.dts.
> >But how should we handle the sleep = <> properties then?
> 
> We could still have some sort of nexus node that is off to the side
> (pointed to with sleep-parent) and not inserting itself into the
> hierarchy.
> 
> Or, we could allow multiple nodes to refer to the same sleep ID, and
> use the ID rather than a node to tie things together.  If we do
> that, we'll probably want a simple index rather than a set of bits
> in the sleep property, so it can correspond to some kernel object
> that has some bookeeping info.  Perhaps we could tie into the clock
> bindings that were discussed on devtree-discuss in August.

Yeah, reusing the clk api would be the best.

Anyway, since we don't use the sleep-nexus stuff, I'd rather
just add the power management controller nodes.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Anton Vorontsov]

Currently 83xx PMC driver clears deep_sleeping variable very early,
before devices are resumed. This makes fsl_deep_sleep() unusable in
drivers' resume() callback.

Sure, drivers can store fsl_deep_sleep() value on suspend and use
the stored value on resume. But a better solution is to postpone
clearing the deep_sleeping variable, i.e. move it into finish()
callback.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/platforms/83xx/suspend.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index d306f07..b0c2619 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -194,7 +194,7 @@ out:
 	return ret;
 }
 
-static void mpc83xx_suspend_finish(void)
+static void mpc83xx_suspend_end(void)
 {
 	deep_sleeping = 0;
 }
@@ -278,7 +278,7 @@ static struct platform_suspend_ops mpc83xx_suspend_ops = {
 	.valid = mpc83xx_suspend_valid,
 	.begin = mpc83xx_suspend_begin,
 	.enter = mpc83xx_suspend_enter,
-	.finish = mpc83xx_suspend_finish,
+	.end = mpc83xx_suspend_end,
 };
 
 static int pmc_probe(struct of_device *ofdev,
-- 
1.6.3.3

Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume

[Kumar Gala]

On Dec 10, 2009, at 12:00 PM, Anton Vorontsov wrote:

> Currently 83xx PMC driver clears deep_sleeping variable very early,
> before devices are resumed. This makes fsl_deep_sleep() unusable in
> drivers' resume() callback.
> 
> Sure, drivers can store fsl_deep_sleep() value on suspend and use
> the stored value on resume. But a better solution is to postpone
> clearing the deep_sleeping variable, i.e. move it into finish()
> callback.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
> arch/powerpc/platforms/83xx/suspend.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)

applied to next

- k