ARM: OMAP4 device off support

arch/arm/mach-omap2/dpll44xx.c +5 -0

+/**
+ * omap4_dpll_print_reg - dump out a single DPLL register value
+ * @dpll_reg: register to dump
+ * @name: name of the register
+ * @tuple: content of the register

arch/arm/mach-omap2/dpll44xx.c +19 -0

+}
+
+/*
+ * omap4_dpll_dump_regs - dump out DPLL registers
+ * @dpll_reg: DPLL to dump
+ *
+ * Dump out the contents of the registers for a DPLL. Called if a
+ * restore for DPLL fails to lock.
+ */
+static void omap4_dpll_dump_regs(struct omap4_dpll_regs *dpll_reg)
+{
+	pr_warn("%s: Unable to lock dpll %s[part=%x inst=%x]:\n",
+		__func__, dpll_reg->name, dpll_reg->mod_partition,
+		dpll_reg->mod_inst);
+	omap4_dpll_print_reg(dpll_reg, "clksel", &dpll_reg->clksel);
+	omap4_dpll_print_reg(dpll_reg, "div_m2", &dpll_reg->div_m2);
+	omap4_dpll_print_reg(dpll_reg, "div_m3", &dpll_reg->div_m3);
+	omap4_dpll_print_reg(dpll_reg, "div_m4", &dpll_reg->div_m4);
+	omap4_dpll_print_reg(dpll_reg, "div_m5", &dpll_reg->div_m5);

Hi Tero,

On 04/20/2012 04:33 AM, Tero Kristo wrote:

[...]

Quoted 5 lines

arch/arm/mach-omap2/dpll44xx.c +5 -0

+/**
+ * omap4_dpll_print_reg - dump out a single DPLL register value
+ * @dpll_reg: register to dump
+ * @name: name of the register
+ * @tuple: content of the register

arch/arm/mach-omap2/dpll44xx.c long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate) · line 316

Quoted 10 lines

arch/arm/mach-omap2/dpll44xx.c +10 -0

+ *
+ * Helper dump function to print out a DPLL register value in case
+ * of restore failures.
+ */
+static void omap4_dpll_print_reg(struct omap4_dpll_regs *dpll_reg, char *name,
+				 struct dpll_reg *tuple)
+{
+	if (tuple->offset)
+		pr_warn("%s - Address offset = 0x%08x, value=0x%08x\n", name,
+			tuple->offset, tuple->val);

Minor nit-pick here ...

1. Offset is 16-bits and so we can just have "offset = 0x%04x"
2. Consider dropping "Address" from "Address offset"
3. Can we be consistent in our spaces for "offset = " and "value="?

Quoted 19 lines

arch/arm/mach-omap2/dpll44xx.c +19 -0

+}
+
+/*
+ * omap4_dpll_dump_regs - dump out DPLL registers
+ * @dpll_reg: DPLL to dump
+ *
+ * Dump out the contents of the registers for a DPLL. Called if a
+ * restore for DPLL fails to lock.
+ */
+static void omap4_dpll_dump_regs(struct omap4_dpll_regs *dpll_reg)
+{
+	pr_warn("%s: Unable to lock dpll %s[part=%x inst=%x]:\n",
+		__func__, dpll_reg->name, dpll_reg->mod_partition,
+		dpll_reg->mod_inst);
+	omap4_dpll_print_reg(dpll_reg, "clksel", &dpll_reg->clksel);
+	omap4_dpll_print_reg(dpll_reg, "div_m2", &dpll_reg->div_m2);
+	omap4_dpll_print_reg(dpll_reg, "div_m3", &dpll_reg->div_m3);
+	omap4_dpll_print_reg(dpll_reg, "div_m4", &dpll_reg->div_m4);
+	omap4_dpll_print_reg(dpll_reg, "div_m5", &dpll_reg->div_m5);

arch/arm/mach-omap2/dpll44xx.c long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate) · line 345

Quoted 10 lines

arch/arm/mach-omap2/dpll44xx.c +10 -0

+	omap4_dpll_print_reg(dpll_reg, "div_m6", &dpll_reg->div_m6);
+	omap4_dpll_print_reg(dpll_reg, "div_m7", &dpll_reg->div_m7);
+	omap4_dpll_print_reg(dpll_reg, "clkdcoldo", &dpll_reg->clkdcoldo);
+	omap4_dpll_print_reg(dpll_reg, "clkmode", &dpll_reg->clkmode);
+	omap4_dpll_print_reg(dpll_reg, "autoidle", &dpll_reg->autoidle);
+	if (dpll_reg->idlest.offset)
+		pr_warn("idlest - Address offset = 0x%08x, before val=0x%08x"
+			" after = 0x%08x\n", dpll_reg->idlest.offset,
+			dpll_reg->idlest.val,
+			omap4_dpll_read_reg(dpll_reg, &dpll_reg->idlest));

Nit-pick ...

1. Same comments as above
2. Consider dropping "Address offset" altogether here as we print "idlest" the offset should be implied.
3. Also can we be consistent in our naming of "before val and "after"? For example, "val before =" and "val now = ".

Quoted 60 lines · Jon Hunter

Hi Tero,

On 04/20/2012 04:33 AM, Tero Kristo wrote:

[...]

Quoted 5 lines

arch/arm/mach-omap2/dpll44xx.c +5 -0

+/**
+ * omap4_dpll_print_reg - dump out a single DPLL register value
+ * @dpll_reg: register to dump
+ * @name: name of the register
+ * @tuple: content of the register

arch/arm/mach-omap2/dpll44xx.c long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate) · line 316

Quoted 10 lines

arch/arm/mach-omap2/dpll44xx.c +10 -0

+ *
+ * Helper dump function to print out a DPLL register value in case
+ * of restore failures.
+ */
+static void omap4_dpll_print_reg(struct omap4_dpll_regs *dpll_reg, char *name,
+				 struct dpll_reg *tuple)
+{
+	if (tuple->offset)
+		pr_warn("%s - Address offset = 0x%08x, value=0x%08x\n", name,
+			tuple->offset, tuple->val);

Minor nit-pick here ...

1. Offset is 16-bits and so we can just have "offset = 0x%04x"
2. Consider dropping "Address" from "Address offset"
3. Can we be consistent in our spaces for "offset = " and "value="?

Quoted 19 lines

arch/arm/mach-omap2/dpll44xx.c +19 -0

+}
+
+/*
+ * omap4_dpll_dump_regs - dump out DPLL registers
+ * @dpll_reg: DPLL to dump
+ *
+ * Dump out the contents of the registers for a DPLL. Called if a
+ * restore for DPLL fails to lock.
+ */
+static void omap4_dpll_dump_regs(struct omap4_dpll_regs *dpll_reg)
+{
+	pr_warn("%s: Unable to lock dpll %s[part=%x inst=%x]:\n",
+		__func__, dpll_reg->name, dpll_reg->mod_partition,
+		dpll_reg->mod_inst);
+	omap4_dpll_print_reg(dpll_reg, "clksel", &dpll_reg->clksel);
+	omap4_dpll_print_reg(dpll_reg, "div_m2", &dpll_reg->div_m2);
+	omap4_dpll_print_reg(dpll_reg, "div_m3", &dpll_reg->div_m3);
+	omap4_dpll_print_reg(dpll_reg, "div_m4", &dpll_reg->div_m4);
+	omap4_dpll_print_reg(dpll_reg, "div_m5", &dpll_reg->div_m5);

arch/arm/mach-omap2/dpll44xx.c long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate) · line 345

Quoted 10 lines

arch/arm/mach-omap2/dpll44xx.c +10 -0

+	omap4_dpll_print_reg(dpll_reg, "div_m6", &dpll_reg->div_m6);
+	omap4_dpll_print_reg(dpll_reg, "div_m7", &dpll_reg->div_m7);
+	omap4_dpll_print_reg(dpll_reg, "clkdcoldo", &dpll_reg->clkdcoldo);
+	omap4_dpll_print_reg(dpll_reg, "clkmode", &dpll_reg->clkmode);
+	omap4_dpll_print_reg(dpll_reg, "autoidle", &dpll_reg->autoidle);
+	if (dpll_reg->idlest.offset)
+		pr_warn("idlest - Address offset = 0x%08x, before val=0x%08x"
+			" after = 0x%08x\n", dpll_reg->idlest.offset,
+			dpll_reg->idlest.val,
+			omap4_dpll_read_reg(dpll_reg, &dpll_reg->idlest));

Nit-pick ...

Quoted 5 lines

1. Same comments as above
2. Consider dropping "Address offset" altogether here as we print "idlest" the offset should be implied.
3. Also can we be consistent in our naming of "before val and "after"? For example, "val before =" and "val now = ".

Okay, I'll modify both prints slightly. Question though, do we want these prints in the kernel in the first place? At least Tony has been frowning upon register dumps in the kernel and this falls into that category. What we could do is just to print the warning but drop the register dumps out.

Hi Tero,

Quoted 67 lines · Tero Kristo

Quoted 65 lines · Jon Hunter

Hi Tero,

On 04/20/2012 04:33 AM, Tero Kristo wrote:

[...]

Quoted 5 lines

arch/arm/mach-omap2/dpll44xx.c +5 -0

+/**
+ * omap4_dpll_print_reg - dump out a single DPLL register value
+ * @dpll_reg: register to dump
+ * @name: name of the register
+ * @tuple: content of the register

arch/arm/mach-omap2/dpll44xx.c long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate) · line 316

Quoted 10 lines

arch/arm/mach-omap2/dpll44xx.c +10 -0

+ *
+ * Helper dump function to print out a DPLL register value in case
+ * of restore failures.
+ */
+static void omap4_dpll_print_reg(struct omap4_dpll_regs *dpll_reg, char *name,
+				 struct dpll_reg *tuple)
+{
+	if (tuple->offset)
+		pr_warn("%s - Address offset = 0x%08x, value=0x%08x\n", name,
+			tuple->offset, tuple->val);

Minor nit-pick here ...

1. Offset is 16-bits and so we can just have "offset = 0x%04x"
2. Consider dropping "Address" from "Address offset"
3. Can we be consistent in our spaces for "offset = " and "value="?

Quoted 19 lines

arch/arm/mach-omap2/dpll44xx.c +19 -0

+}
+
+/*
+ * omap4_dpll_dump_regs - dump out DPLL registers
+ * @dpll_reg: DPLL to dump
+ *
+ * Dump out the contents of the registers for a DPLL. Called if a
+ * restore for DPLL fails to lock.
+ */
+static void omap4_dpll_dump_regs(struct omap4_dpll_regs *dpll_reg)
+{
+	pr_warn("%s: Unable to lock dpll %s[part=%x inst=%x]:\n",
+		__func__, dpll_reg->name, dpll_reg->mod_partition,
+		dpll_reg->mod_inst);
+	omap4_dpll_print_reg(dpll_reg, "clksel", &dpll_reg->clksel);
+	omap4_dpll_print_reg(dpll_reg, "div_m2", &dpll_reg->div_m2);
+	omap4_dpll_print_reg(dpll_reg, "div_m3", &dpll_reg->div_m3);
+	omap4_dpll_print_reg(dpll_reg, "div_m4", &dpll_reg->div_m4);
+	omap4_dpll_print_reg(dpll_reg, "div_m5", &dpll_reg->div_m5);

arch/arm/mach-omap2/dpll44xx.c long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate) · line 345

Quoted 10 lines

arch/arm/mach-omap2/dpll44xx.c +10 -0

+	omap4_dpll_print_reg(dpll_reg, "div_m6", &dpll_reg->div_m6);
+	omap4_dpll_print_reg(dpll_reg, "div_m7", &dpll_reg->div_m7);
+	omap4_dpll_print_reg(dpll_reg, "clkdcoldo", &dpll_reg->clkdcoldo);
+	omap4_dpll_print_reg(dpll_reg, "clkmode", &dpll_reg->clkmode);
+	omap4_dpll_print_reg(dpll_reg, "autoidle", &dpll_reg->autoidle);
+	if (dpll_reg->idlest.offset)
+		pr_warn("idlest - Address offset = 0x%08x, before val=0x%08x"
+			" after = 0x%08x\n", dpll_reg->idlest.offset,
+			dpll_reg->idlest.val,
+			omap4_dpll_read_reg(dpll_reg, &dpll_reg->idlest));

Nit-pick ...

1. Same comments as above
2. Consider dropping "Address offset" altogether here as we print "idlest" the offset should be implied.
3. Also can we be consistent in our naming of "before val and "after"? For example, "val before =" and "val now = ".

Quoted 5 lines

Okay, I'll modify both prints slightly. Question though, do we want these prints in the kernel in the first place? At least Tony has been frowning upon register dumps in the kernel and this falls into that category. What we could do is just to print the warning but drop the register dumps out.

Thanks. Good question. If we are not seeing failures often, and I would hope not, probably ok to remove. However, I will let Tony comment here ...

From: Rajendra Nayak <[email protected]>

SAR/ROM code restores only CORE DPLL to its original state post wakeup from OFF mode.
The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER) are saved and restored here during an OFF transition.

[nm at ti.com: minor cleanups]

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/cm44xx.h   |    5 +
 arch/arm/mach-omap2/dpll44xx.c |  271 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 276 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/cm44xx.h +5 -0

index 3380bee..5fba0fa 100644
@@ -23,4 +23,9 @@
 #define OMAP4_CM_CLKSTCTRL				0x0000
 #define OMAP4_CM_STATICDEP				0x0004
 
+#ifndef __ASSEMBLER__
+extern void omap4_dpll_prepare_off(void);
+extern void omap4_dpll_resume_off(void);
+#endif
+
 #endif

arch/arm/mach-omap2/dpll44xx.c +261 -0

index 9c6a296..b411d3b 100644
@@ -14,6 +14,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/bitops.h>
+#include <linux/delay.h>
 
 #include <plat/cpu.h>
 #include <plat/clock.h>

@@ -21,6 +22,96 @@
 #include "clock.h"
 #include "clock44xx.h"
 #include "cm-regbits-44xx.h"
+#include "cm1_44xx.h"
+#include "cm2_44xx.h"
+#include "prcm44xx.h"
+#include "cminst44xx.h"
+#include "cm44xx.h"
+
+#define MAX_DPLL_WAIT_TRIES	1000000
+
+struct dpll_reg {
+	u16 offset;
+	u32 val;
+};
+
+struct omap4_dpll_regs {
+	char *name;
+	u32 mod_partition;
+	u32 mod_inst;
+	struct dpll_reg clkmode;
+	struct dpll_reg autoidle;
+	struct dpll_reg idlest;
+	struct dpll_reg clksel;
+	struct dpll_reg div_m2;
+	struct dpll_reg div_m3;
+	struct dpll_reg div_m4;
+	struct dpll_reg div_m5;
+	struct dpll_reg div_m6;
+	struct dpll_reg div_m7;
+	struct dpll_reg clkdcoldo;
+};
+
+static struct omap4_dpll_regs dpll_regs[] = {
+	/* MPU DPLL */
+	{ .name		= "mpu",
+	  .mod_partition = OMAP4430_CM1_PARTITION,
+	  .mod_inst	= OMAP4430_CM1_CKGEN_INST,
+	  .clkmode	= {.offset = OMAP4_CM_CLKMODE_DPLL_MPU_OFFSET},
+	  .autoidle	= {.offset = OMAP4_CM_AUTOIDLE_DPLL_MPU_OFFSET},
+	  .idlest	= {.offset = OMAP4_CM_IDLEST_DPLL_MPU_OFFSET},
+	  .clksel	= {.offset = OMAP4_CM_CLKSEL_DPLL_MPU_OFFSET},
+	  .div_m2	= {.offset = OMAP4_CM_DIV_M2_DPLL_MPU_OFFSET},
+	},
+	/* IVA DPLL */
+	{ .name		= "iva",
+	  .mod_partition = OMAP4430_CM1_PARTITION,
+	  .mod_inst	= OMAP4430_CM1_CKGEN_INST,
+	  .clkmode	= {.offset = OMAP4_CM_CLKMODE_DPLL_IVA_OFFSET},
+	  .autoidle	= {.offset = OMAP4_CM_AUTOIDLE_DPLL_IVA_OFFSET},
+	  .idlest	= {.offset = OMAP4_CM_IDLEST_DPLL_IVA_OFFSET},
+	  .clksel	= {.offset = OMAP4_CM_CLKSEL_DPLL_IVA_OFFSET},
+	  .div_m4	= {.offset = OMAP4_CM_DIV_M4_DPLL_IVA_OFFSET},
+	  .div_m5	= {.offset = OMAP4_CM_DIV_M5_DPLL_IVA_OFFSET},
+	},
+	/* ABE DPLL */
+	{ .name		= "abe",
+	  .mod_partition = OMAP4430_CM1_PARTITION,
+	  .mod_inst	= OMAP4430_CM1_CKGEN_INST,
+	  .clkmode	= {.offset = OMAP4_CM_CLKMODE_DPLL_ABE_OFFSET},
+	  .autoidle	= {.offset = OMAP4_CM_AUTOIDLE_DPLL_ABE_OFFSET},
+	  .idlest	= {.offset = OMAP4_CM_IDLEST_DPLL_ABE_OFFSET},
+	  .clksel	= {.offset = OMAP4_CM_CLKSEL_DPLL_ABE_OFFSET},
+	  .div_m2	= {.offset = OMAP4_CM_DIV_M2_DPLL_ABE_OFFSET},
+	  .div_m3	= {.offset = OMAP4_CM_DIV_M3_DPLL_ABE_OFFSET},
+	},
+	/* USB DPLL */
+	{ .name		= "usb",
+	  .mod_partition = OMAP4430_CM2_PARTITION,
+	  .mod_inst	= OMAP4430_CM2_CKGEN_INST,
+	  .clkmode	= {.offset = OMAP4_CM_CLKMODE_DPLL_USB_OFFSET},
+	  .autoidle	= {.offset = OMAP4_CM_AUTOIDLE_DPLL_USB_OFFSET},
+	  .idlest	= {.offset = OMAP4_CM_IDLEST_DPLL_USB_OFFSET},
+	  .clksel	= {.offset = OMAP4_CM_CLKSEL_DPLL_USB_OFFSET},
+	  .div_m2	= {.offset = OMAP4_CM_DIV_M2_DPLL_USB_OFFSET},
+	  .clkdcoldo	= {.offset = OMAP4_CM_CLKDCOLDO_DPLL_USB_OFFSET},
+	 },
+	/* PER DPLL */
+	{ .name		= "per",
+	  .mod_partition = OMAP4430_CM2_PARTITION,
+	  .mod_inst	= OMAP4430_CM2_CKGEN_INST,
+	  .clkmode	= {.offset = OMAP4_CM_CLKMODE_DPLL_PER_OFFSET},
+	  .autoidle	= {.offset = OMAP4_CM_AUTOIDLE_DPLL_PER_OFFSET},
+	  .idlest	= {.offset = OMAP4_CM_IDLEST_DPLL_PER_OFFSET},
+	  .clksel	= {.offset = OMAP4_CM_CLKSEL_DPLL_PER_OFFSET},
+	  .div_m2	= {.offset = OMAP4_CM_DIV_M2_DPLL_PER_OFFSET},
+	  .div_m3	= {.offset = OMAP4_CM_DIV_M3_DPLL_PER_OFFSET},
+	  .div_m4	= {.offset = OMAP4_CM_DIV_M4_DPLL_PER_OFFSET},
+	  .div_m5	= {.offset = OMAP4_CM_DIV_M5_DPLL_PER_OFFSET},
+	  .div_m6	= {.offset = OMAP4_CM_DIV_M6_DPLL_PER_OFFSET},
+	  .div_m7	= {.offset = OMAP4_CM_DIV_M7_DPLL_PER_OFFSET},
+	},
+};
 
 /* Supported only on OMAP4 */
 int omap4_dpllmx_gatectrl_read(struct clk *clk)

@@ -151,3 +242,183 @@ long omap4_dpll_regm4xen_round_rate(struct clk *clk, unsigned long target_rate)
 
 	return clk->dpll_data->last_rounded_rate;
 }
+
+/**
+ * omap4_dpll_read_reg - reads DPLL register value
+ * @dpll_reg: DPLL register to read
+ *
+ * Reads the value of a single DPLL register.
+ */
+static inline u32 omap4_dpll_read_reg(struct omap4_dpll_regs *dpll_reg,
+				      struct dpll_reg *tuple)
+{
+	if (tuple->offset)
+		return omap4_cminst_read_inst_reg(dpll_reg->mod_partition,
+						  dpll_reg->mod_inst,
+						  tuple->offset);
+	return 0;
+}
+
+/**
+ * omap4_dpll_store_reg - stores DPLL register value to memory location
+ * @dpll_reg: DPLL register to save
+ * @tuple: save address
+ *
+ * Saves a single DPLL register content to memory location defined by
+ * @tuple before entering device off mode.
+ */
+static inline void omap4_dpll_store_reg(struct omap4_dpll_regs *dpll_reg,
+					struct dpll_reg *tuple)
+{
+	tuple->val = omap4_dpll_read_reg(dpll_reg, tuple);
+}
+
+/**
+ * omap4_dpll_prepare_off - stores DPLL settings before off mode
+ *
+ * Saves all DPLL register settings. This must be called before
+ * entering device off.
+ */
+void omap4_dpll_prepare_off(void)
+{
+	u32 i;
+	struct omap4_dpll_regs *dpll_reg = dpll_regs;
+
+	for (i = 0; i < ARRAY_SIZE(dpll_regs); i++, dpll_reg++) {
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->clkmode);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->autoidle);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->clksel);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->div_m2);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->div_m3);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->div_m4);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->div_m5);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->div_m6);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->div_m7);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->clkdcoldo);
+		omap4_dpll_store_reg(dpll_reg, &dpll_reg->idlest);
+	}
+}
+
+/**
+ * omap4_dpll_print_reg - dump out a single DPLL register value
+ * @dpll_reg: register to dump
+ * @name: name of the register
+ * @tuple: content of the register
+ *
+ * Helper dump function to print out a DPLL register value in case
+ * of restore failures.
+ */
+static void omap4_dpll_print_reg(struct omap4_dpll_regs *dpll_reg, char *name,
+				 struct dpll_reg *tuple)
+{
+	if (tuple->offset)
+		pr_warn("%s - Address offset = 0x%08x, value=0x%08x\n", name,
+			tuple->offset, tuple->val);
+}
+
+/*
+ * omap4_dpll_dump_regs - dump out DPLL registers
+ * @dpll_reg: DPLL to dump
+ *
+ * Dump out the contents of the registers for a DPLL. Called if a
+ * restore for DPLL fails to lock.
+ */
+static void omap4_dpll_dump_regs(struct omap4_dpll_regs *dpll_reg)
+{
+	pr_warn("%s: Unable to lock dpll %s[part=%x inst=%x]:\n",
+		__func__, dpll_reg->name, dpll_reg->mod_partition,
+		dpll_reg->mod_inst);
+	omap4_dpll_print_reg(dpll_reg, "clksel", &dpll_reg->clksel);
+	omap4_dpll_print_reg(dpll_reg, "div_m2", &dpll_reg->div_m2);
+	omap4_dpll_print_reg(dpll_reg, "div_m3", &dpll_reg->div_m3);
+	omap4_dpll_print_reg(dpll_reg, "div_m4", &dpll_reg->div_m4);
+	omap4_dpll_print_reg(dpll_reg, "div_m5", &dpll_reg->div_m5);
+	omap4_dpll_print_reg(dpll_reg, "div_m6", &dpll_reg->div_m6);
+	omap4_dpll_print_reg(dpll_reg, "div_m7", &dpll_reg->div_m7);
+	omap4_dpll_print_reg(dpll_reg, "clkdcoldo", &dpll_reg->clkdcoldo);
+	omap4_dpll_print_reg(dpll_reg, "clkmode", &dpll_reg->clkmode);
+	omap4_dpll_print_reg(dpll_reg, "autoidle", &dpll_reg->autoidle);
+	if (dpll_reg->idlest.offset)
+		pr_warn("idlest - Address offset = 0x%08x, before val=0x%08x"
+			" after = 0x%08x\n", dpll_reg->idlest.offset,
+			dpll_reg->idlest.val,
+			omap4_dpll_read_reg(dpll_reg, &dpll_reg->idlest));
+}
+
+/**
+ * omap4_wait_dpll_lock - wait for a DPLL lock
+ * @dpll_reg: DPLL to wait for
+ *
+ * Waits for a DPLL lock after restore.
+ */
+static void omap4_wait_dpll_lock(struct omap4_dpll_regs *dpll_reg)
+{
+	int j = 0;
+	u32 status;
+
+	/* Return if we dont need to lock. */
+	if ((dpll_reg->clkmode.val & OMAP4430_DPLL_EN_MASK) !=
+	     DPLL_LOCKED << OMAP4430_DPLL_EN_SHIFT)
+		return;
+
+	while (1) {
+		status = (omap4_dpll_read_reg(dpll_reg, &dpll_reg->idlest)
+			  & OMAP4430_ST_DPLL_CLK_MASK)
+			 >> OMAP4430_ST_DPLL_CLK_SHIFT;
+		if (status == 0x1)
+			break;
+		if (j == MAX_DPLL_WAIT_TRIES) {
+			/* If we are unable to lock, warn and move on.. */
+			omap4_dpll_dump_regs(dpll_reg);
+			break;
+		}
+		j++;
+		udelay(1);
+	}
+}
+
+/**
+ * omap4_dpll_restore_reg - restores a single register for a DPLL
+ * @dpll_reg: DPLL to restore
+ * @tuple: register value to restore
+ *
+ * Restores a single register for a DPLL.
+ */
+static inline void omap4_dpll_restore_reg(struct omap4_dpll_regs *dpll_reg,
+					  struct dpll_reg *tuple)
+{
+	if (tuple->offset)
+		omap4_cminst_write_inst_reg(tuple->val, dpll_reg->mod_partition,
+					    dpll_reg->mod_inst, tuple->offset);
+}
+
+/**
+ * omap4_dpll_resume_off - restore DPLL settings after device off
+ *
+ * Restores all DPLL settings. Must be called after wakeup from device
+ * off.
+ */
+void omap4_dpll_resume_off(void)
+{
+	u32 i;
+	struct omap4_dpll_regs *dpll_reg = dpll_regs;
+
+	for (i = 0; i < ARRAY_SIZE(dpll_regs); i++, dpll_reg++) {
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clksel);
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m2);
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m3);
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m4);
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m5);
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m6);
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m7);
+		omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clkdcoldo);

Hi Tero,

Quoted 7 lines · Tero

From: Rajendra Nayak <[email protected]>

SAR/ROM code restores only CORE DPLL to its original state post wakeup from OFF mode.
The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER) are saved and restored here during an OFF transition.

Most of the registers being saved and restored in the various patches look to be contiguous. So, instead of a long list of these contiguous registers how about having a generic API for backup and restore of the registers contents based on the memory address range?

Most of the registers which require save and restore are either part of PRM, CM or Control. The PRM or CM instance could be passed as the base and the save/restore API could work around that.

One downside is that there are some read-only registers in between but I don't see any simple way of avoiding save and restore of those. BTW, as per the TRM that I have, this patch is also doing save and restore of some read-only registers.

Quoted 19 lines

(quoted diff) +19 -0

+void omap4_dpll_resume_off(void)
+{
+ ? ? u32 i;
+ ? ? struct omap4_dpll_regs *dpll_reg = dpll_regs;
+
+ ? ? for (i = 0; i < ARRAY_SIZE(dpll_regs); i++, dpll_reg++) {
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clksel);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m2);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m3);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m4);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m5);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m6);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m7);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clkdcoldo);
+
+ ? ? ? ? ? ? /* Restore clkmode after the above registers are restored */
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clkmode);
+
+ ? ? ? ? ? ? omap4_wait_dpll_lock(dpll_reg);

Review

Quoted 5 lines

(quoted diff) +5 -0

+
+ ? ? ? ? ? ? /* Restore autoidle settings after the dpll is locked */
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->autoidle);
+ ? ? }
+}

If this function is placed in SRAM and PER PLL restored just after MPU won't the exit latency be further optimized?

Quoted 20 lines · Vaibhav

Hi Tero,

Quoted 7 lines · Tero

From: Rajendra Nayak <[email protected]>

SAR/ROM code restores only CORE DPLL to its original state post wakeup from OFF mode.
The rest of the DPLL's in OMAP4 platform (MPU/IVA/ABE/USB/PER) are saved and restored here during an OFF transition.

Most of the registers being saved and restored in the various patches look to be contiguous. So, instead of a long list of these contiguous registers how about having a generic API for backup and restore of the registers contents based on the memory address range?

Most of the registers which require save and restore are either part of PRM, CM or Control. The PRM or CM instance could be passed as the base and the save/restore API could work around that.

Quoted 5 lines

One downside is that there are some read-only registers in between but I don't see any simple way of avoiding save and restore of those. BTW, as per the TRM that I have, this patch is also doing save and restore of some read-only registers.

You should never write to read-only registers since the behavior is undefined.

[...]

Quoted 19 lines

Quoted 19 lines

(quoted diff) +19 -0

+void omap4_dpll_resume_off(void)
+{
+ ? ? u32 i;
+ ? ? struct omap4_dpll_regs *dpll_reg = dpll_regs;
+
+ ? ? for (i = 0; i < ARRAY_SIZE(dpll_regs); i++, dpll_reg++) {
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clksel);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m2);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m3);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m4);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m5);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m6);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m7);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clkdcoldo);
+
+ ? ? ? ? ? ? /* Restore clkmode after the above registers are restored */
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clkmode);
+
+ ? ? ? ? ? ? omap4_wait_dpll_lock(dpll_reg);

Quoted 9 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+
+ ? ? ? ? ? ? /* Restore autoidle settings after the dpll is locked */
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->autoidle);
+ ? ? }
+}

If this function is placed in SRAM and PER PLL restored just after MPU won't the exit latency be further optimized?

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

Quoted 6 lines · Santosh

Quoted 5 lines · Vaibhav

Hi Tero,

Quoted 2 lines · Tero

From: Rajendra Nayak <[email protected]>

Quoted 30 lines

[...]

Quoted 28 lines

Quoted 14 lines

(quoted diff) +14 -0

+void omap4_dpll_resume_off(void)
+{
+ ? ? u32 i;
+ ? ? struct omap4_dpll_regs *dpll_reg = dpll_regs;
+
+ ? ? for (i = 0; i < ARRAY_SIZE(dpll_regs); i++, dpll_reg++) {
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clksel);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m2);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m3);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m4);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m5);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m6);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->div_m7);
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clkdcoldo);

Review

Quoted 10 lines

(quoted diff) +10 -0

+
+ ? ? ? ? ? ? /* Restore clkmode after the above registers are restored */
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->clkmode);
+
+ ? ? ? ? ? ? omap4_wait_dpll_lock(dpll_reg);
+
+ ? ? ? ? ? ? /* Restore autoidle settings after the dpll is locked */
+ ? ? ? ? ? ? omap4_dpll_restore_reg(dpll_reg, &dpll_reg->autoidle);
+ ? ? }
+}

If this function is placed in SRAM and PER PLL restored just after MPU won't the exit latency be further optimized?

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

Quoted 5 lines

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

Quoted 3 lines

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

You don't have to do any real measurements at least on OMAP. OCMC RAM is interfaced over L4 and MPU has to cross two interconnect bridges to reach to SRAM. DDR is more of direct path and much faster.

On Wed, May 02, 2012 at 16:30:26, Shilimkar, Santosh wrote:
[...]

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

Quoted 4 lines

You don't have to do any real measurements at least on OMAP. OCMC RAM is interfaced over L4 and MPU has to cross two interconnect bridges to reach to SRAM. DDR is more of direct path and much faster.

Hmm, I was under the impression that OCMC RAM was on L3, at least for OMAP4.

Maybe there's a extra low latency path for DDR that I am missing.

Quoted 16 lines · Vaibhav

On Wed, May 02, 2012 at 16:30:26, Shilimkar, Santosh wrote:
[...]

Quoted 12 lines

Quoted 8 lines

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

You don't have to do any real measurements at least on OMAP. OCMC RAM is interfaced over L4 and MPU has to cross two interconnect bridges to reach to SRAM. DDR is more of direct path and much faster.

Quoted 1 line

Hmm, I was under the impression that OCMC RAM was on L3, at least for OMAP4.

Nope. It's on L4 and it is same for all OMAPs including OMAP5

Quoted 1 line

Maybe there's a extra low latency path for DDR that I am missing.

Even OMAP3 DDR is faster than OCMC RAM.

On Wed, May 02, 2012 at 16:30:26, Shilimkar, Santosh wrote:
[...]

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

Quoted 4 lines

You don't have to do any real measurements at least on OMAP. OCMC RAM is interfaced over L4 and MPU has to cross two interconnect bridges to reach to SRAM. DDR is more of direct path and much faster.

Hmm, I was under the impression that OCMC RAM was on L3, at least for OMAP4.
Maybe there's a extra low latency path for DDR that I am missing.

Quoted 13 lines · Vaibhav

On Wed, May 02, 2012 at 16:30:26, Shilimkar, Santosh wrote:
[...]

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

Quoted 8 lines

Quoted 4 lines

You don't have to do any real measurements at least on OMAP. OCMC RAM is interfaced over L4 and MPU has to cross two interconnect bridges to reach to SRAM. DDR is more of direct path and much faster.

Hmm, I was under the impression that OCMC RAM was on L3, at least for OMAP4.
Maybe there's a extra low latency path for DDR that I am missing.

have you folks considered the possibility that SRAM may be reprogrammed by Security code to almost nothing if PA/PPA size needs to be increased? it would be rather dumb not to support HS devices on which real products are made, no? rule of thumb has been to avoid usage of SRAM as it constraints security folks as well (yep, I know they should share code with all of us etc.. but they have a different sets of challenges that may deny them such luxury).

Quoted 22 lines · Nishanth

Quoted 13 lines · Vaibhav

On Wed, May 02, 2012 at 16:30:26, Shilimkar, Santosh wrote:
[...]

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

Quoted 8 lines

Quoted 4 lines

You don't have to do any real measurements at least on OMAP. OCMC RAM is interfaced over L4 and MPU has to cross two interconnect bridges to reach to SRAM. DDR is more of direct path and much faster.

Hmm, I was under the impression that OCMC RAM was on L3, at least for OMAP4.
Maybe there's a extra low latency path for DDR that I am missing.

Quoted 8 lines

have you folks considered the possibility that SRAM may be reprogrammed by Security code to almost nothing if PA/PPA size needs to be increased? it would be rather dumb not to support HS devices on which real products are made, no? rule of thumb has been to avoid usage of SRAM as it constraints security folks as well (yep, I know they should share code with all of us etc.. but they have a different sets of challenges that may deny them such luxury).

Even I would like to avoid the usage of MPUSS SRAM for this very reason. But AFAIK the whole PPA stuff isn't applicable for OCMC RAM (not SRAM inside MPUSS).

Quoted 32 lines · Vaibhav

Quoted 30 lines · Nishanth

Quoted 13 lines · Vaibhav

On Wed, May 02, 2012 at 16:30:26, Shilimkar, Santosh wrote:
[...]

Quoted 9 lines

Quoted 9 lines

Quoted 4 lines

How ?
SRAM is sower memory than DDR so I don't see how it will reduce latency.

I am just guessing if that's indeed the case ;)
Haven't done any measurements to really check if that's indeed the case though.

Quoted 8 lines

Quoted 4 lines

You don't have to do any real measurements at least on OMAP. OCMC RAM is interfaced over L4 and MPU has to cross two interconnect bridges to reach to SRAM. DDR is more of direct path and much faster.

Hmm, I was under the impression that OCMC RAM was on L3, at least for OMAP4.
Maybe there's a extra low latency path for DDR that I am missing.

have you folks considered the possibility that SRAM may be reprogrammed by Security code to almost nothing if PA/PPA size needs to be increased? it would be rather dumb not to support HS devices on which real products are made, no? rule of thumb has been to avoid usage of SRAM as it constraints security folks as well (yep, I know they should share code with all of us etc.. but they have a different sets of challenges that may deny them such luxury).

Quoted 2 lines

Even I would like to avoid the usage of MPUSS SRAM for this very reason. But AFAIK the whole PPA stuff isn't applicable for OCMC RAM (not SRAM inside MPUSS).

On OMAP, even OCMC RAM can be used by security middle-ware apart from reserved SRAM
for HS/EMU devices.
That's what Nishant meant.

From: Rajendra Nayak <[email protected]>

Restore all CM1/2 module registers as they are lost in OFF mode.

[nm at ti.com: minor clean ups]

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

Signed-off-by Axel Haslam <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/cm44xx.c |  322 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/cm44xx.h |    2 +
 2 files changed, 324 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/cm44xx.c +4 -0

index 535d66e..fb5465b 100644
@@ -21,8 +21,11 @@
 #include "iomap.h"
 #include "common.h"
 #include "cm.h"
+#include "cm44xx.h"
 #include "cm1_44xx.h"
 #include "cm2_44xx.h"
+#include "cminst44xx.h"
+#include "prcm44xx.h"
 #include "cm-regbits-44xx.h"
 
 /* CM1 hardware module low-level functions */

@@ -50,3 +53,322 @@ void omap4_cm2_write_inst_reg(u32 val, s16 inst, u16 reg)
 {
 	__raw_writel(val, OMAP44XX_CM2_REGADDR(inst, reg));
 }
+

arch/arm/mach-omap2/cm44xx.c +1 -0

+	},

arch/arm/mach-omap2/cm44xx.c +48 -0

+static void omap4_cm1_prepare_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm1_regs;
+
+	for (i = 0; i < ARRAY_SIZE(cm1_regs); i++, cm_reg++) {
+		for (j = 0; j < cm_reg->no_reg; j++) {
+			cm_reg->reg[j].val =
+			    omap4_cminst_read_inst_reg(OMAP4430_CM1_PARTITION,
+						       cm_reg->mod_off,
+						       cm_reg->reg[j].offset);
+		}
+	}
+}
+
+static void omap4_cm2_prepare_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm2_regs;
+
+	for (i = 0; i < ARRAY_SIZE(cm2_regs); i++, cm_reg++) {
+		for (j = 0; j < cm_reg->no_reg; j++) {
+			cm_reg->reg[j].val =
+			    omap4_cminst_read_inst_reg(OMAP4430_CM2_PARTITION,
+						       cm_reg->mod_off,
+						       cm_reg->reg[j].offset);
+		}
+	}
+}
 
+static void omap4_cm1_resume_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm1_regs;
+
+	for (i = 0; i < ARRAY_SIZE(cm1_regs); i++, cm_reg++) {
+		for (j = 0; j < cm_reg->no_reg; j++) {
+			omap4_cminst_write_inst_reg(cm_reg->reg[j].val,
+						    OMAP4430_CM1_PARTITION,
+						    cm_reg->mod_off,
+						    cm_reg->reg[j].offset);
+		}
+	}
+}
+
+static void omap4_cm2_resume_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm2_regs;

Quoted 3 lines · Tero Kristo

From: Rajendra Nayak <[email protected]>

Restore all CM1/2 module registers as they are lost in OFF mode.

Except they are still lost since nobody calls these new functions (in this patch.) :)

For ease of review, it's preferred to add the *users* of new code in the same patch as the new code.

Quoted 37 lines

[nm at ti.com: minor clean ups]

Signed-off-by Nishanth Menon <[email protected]>

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

Signed-off-by Axel Haslam <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/cm44xx.c |  322 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/cm44xx.h |    2 +
 2 files changed, 324 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/cm44xx.c +8 -0

index 535d66e..fb5465b 100644
@@ -21,8 +21,11 @@
 #include "iomap.h"
 #include "common.h"
 #include "cm.h"
+#include "cm44xx.h"
 #include "cm1_44xx.h"
 #include "cm2_44xx.h"
+#include "cminst44xx.h"
+#include "prcm44xx.h"
 #include "cm-regbits-44xx.h"
 
 /* CM1 hardware module low-level functions */

@@ -50,3 +53,322 @@ void omap4_cm2_write_inst_reg(u32 val, s16 inst, u16 reg)
 {
 	__raw_writel(val, OMAP44XX_CM2_REGADDR(inst, reg));
 }
+
+#define MAX_CM_REGISTERS 51
+
+struct omap4_cm_reg {
+	u16 offset;

arch/arm/mach-omap2/cm44xx.c void omap4_cm2_write_inst_reg(u32 val, s16 inst, u16 reg) · line 66

Quoted 6 lines

arch/arm/mach-omap2/cm44xx.c +6 -0

+	u32 val;
+};
+
+struct omap4_cm_regs {
+	u32 mod_off;
+	u32 no_reg;

minor: do these need to be u32?

arch/arm/mach-omap2/cm44xx.c void omap4_cm2_write_inst_reg(u32 val, s16 inst, u16 reg) · line 73

Quoted 8 lines

arch/arm/mach-omap2/cm44xx.c +7 -0

+	struct omap4_cm_reg reg[MAX_CM_REGISTERS];
+};
+
+static struct omap4_cm_regs cm1_regs[] = {
+	/* OMAP4430_CM1_OCP_SOCKET_MOD */
+	{ .mod_off = OMAP4430_CM1_OCP_SOCKET_INST, .no_reg = 1,
+	{{.offset = OMAP4_CM_CM1_PROFILING_CLKCTRL_OFFSET} },

For readability sake, I'd prefer to see this line indented. And why the extra space before the final '}'}

Quoted 2 lines

arch/arm/mach-omap2/cm44xx.c +1 -0

+	},

[...]

Quoted 49 lines

arch/arm/mach-omap2/cm44xx.c +48 -0

+static void omap4_cm1_prepare_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm1_regs;
+
+	for (i = 0; i < ARRAY_SIZE(cm1_regs); i++, cm_reg++) {
+		for (j = 0; j < cm_reg->no_reg; j++) {
+			cm_reg->reg[j].val =
+			    omap4_cminst_read_inst_reg(OMAP4430_CM1_PARTITION,
+						       cm_reg->mod_off,
+						       cm_reg->reg[j].offset);
+		}
+	}
+}
+
+static void omap4_cm2_prepare_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm2_regs;
+
+	for (i = 0; i < ARRAY_SIZE(cm2_regs); i++, cm_reg++) {
+		for (j = 0; j < cm_reg->no_reg; j++) {
+			cm_reg->reg[j].val =
+			    omap4_cminst_read_inst_reg(OMAP4430_CM2_PARTITION,
+						       cm_reg->mod_off,
+						       cm_reg->reg[j].offset);
+		}
+	}
+}
 
+static void omap4_cm1_resume_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm1_regs;
+
+	for (i = 0; i < ARRAY_SIZE(cm1_regs); i++, cm_reg++) {
+		for (j = 0; j < cm_reg->no_reg; j++) {
+			omap4_cminst_write_inst_reg(cm_reg->reg[j].val,
+						    OMAP4430_CM1_PARTITION,
+						    cm_reg->mod_off,
+						    cm_reg->reg[j].offset);
+		}
+	}
+}
+
+static void omap4_cm2_resume_off(void)
+{
+	u32 i, j;
+	struct omap4_cm_regs *cm_reg = cm2_regs;

arch/arm/mach-omap2/cm44xx.c void omap4_cm2_write_inst_reg(u32 val, s16 inst, u16 reg) · line 362

Quoted 10 lines

arch/arm/mach-omap2/cm44xx.c +10 -0

+
+	for (i = 0; i < ARRAY_SIZE(cm2_regs); i++, cm_reg++) {
+		for (j = 0; j < cm_reg->no_reg; j++) {
+			omap4_cminst_write_inst_reg(cm_reg->reg[j].val,
+						    OMAP4430_CM2_PARTITION,
+						    cm_reg->mod_off,
+						    cm_reg->reg[j].offset);
+		}
+	}
+}

Notice the two prpare functions (and resume functions) are basically identical, except for the partition.

How about adding a .partition field to the struct so there can be a single function}

arch/arm/mach-omap2/omap-sar.c new file +9 -0

+
+static int omap4_sar_not_accessible(void)
+{
+	u32 usbhost_state, usbtll_state;
+
+	/*
+	 * Make sure that USB host and TLL modules are not
+	 * enabled before attempting to save the context
+	 * registers, otherwise this will trigger an exception.

Quoted 9 lines · Tero Kristo

arch/arm/mach-omap2/omap-sar.c new file +9 -0

+
+static int omap4_sar_not_accessible(void)
+{
+	u32 usbhost_state, usbtll_state;
+
+	/*
+	 * Make sure that USB host and TLL modules are not
+	 * enabled before attempting to save the context
+	 * registers, otherwise this will trigger an exception.

arch/arm/mach-omap2/omap-sar.c line 126

Quoted 10 lines

arch/arm/mach-omap2/omap-sar.c new file +10 -0

+	 */
+	usbhost_state = omap4_cminst_read_inst_reg(OMAP4430_CM2_PARTITION,
+						   OMAP4430_CM2_L3INIT_INST,
+						   OMAP4_CM_L3INIT_USB_HOST_CLKCTRL_OFFSET)
+	    & (OMAP4430_STBYST_MASK | OMAP4430_IDLEST_MASK);
+
+	usbtll_state = omap4_cminst_read_inst_reg(OMAP4430_CM2_PARTITION,
+						  OMAP4430_CM2_L3INIT_INST,
+						  OMAP4_CM_L3INIT_USB_TLL_CLKCTRL_OFFSET)
+	    & OMAP4430_IDLEST_MASK;

Formatting here looks bad, maybe just do it in separate steps:

	usbhost_state = omap4_cminst...
	usbhost_state &= OMAP4430_STBYST_MASK | OMAP4430_IDLEST_MASK;

...

arch/arm/mach-omap2/omap-sar.c line 149

Quoted 7 lines

arch/arm/mach-omap2/omap-sar.c new file +7 -0

+	/*
+	 * Not supported on ES1.0 silicon
+	 */
+	if (omap_rev() == OMAP4430_REV_ES1_0) {
+		WARN_ONCE(1, "omap4: SAR backup not supported on ES1.0 ..\n");
+		return -ENODEV;
+	}

Everytime there's SoC/hardware revision detection in a non __init function, something is most likely wrong.

arch/arm/mach-omap2/omap-sar.c line 203

Quoted 7 lines

arch/arm/mach-omap2/omap-sar.c new file +7 -0

+void omap4_sar_overwrite(void)
+{
+	u32 val = 0;
+	u32 offset = 0;
+
+	if (cpu_is_omap446x())
+		offset = 0x04;

Here too.

arch/arm/mach-omap2/omap-sar.c line 267

Quoted 8 lines

arch/arm/mach-omap2/omap-sar.c new file +8 -0

+static int __init omap4_sar_ram_init(void)
+{
+	/*
+	 * To avoid code running on other OMAPs in
+	 * multi-omap builds
+	 */
+	if (!cpu_is_omap44xx())
+		return -ENODEV;

You should configure things here instead by setting function pointers or variables.

Regards,

Tony

arch/arm/mach-omap2/omap-sar.c +1 -1

-void omap4_sar_overwrite(void)
+void omap_sar_overwrite(void)
 {
 	u32 val = 0;

This patch adds device off support to OMAP4 device type.

OFF mode is disabled by default, however, there are two ways to enable OFF mode:
a) In the board file, call omap4_pm_off_mode_enable(1)
b) Enable OFF mode using the debugfs entry
echo "1">/sys/kernel/debug/pm_debug/enable_off_mode (conversely echo '0' will disable it as well).

Signed-off-by Santosh Shilimkar <[email protected]>

[t-kristo at ti.com: largely re-structured the code]

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |   24 ++++++++++-
 arch/arm/mach-omap2/omap-wakeupgen.c      |   47 +++++++++++++++++++-
 arch/arm/mach-omap2/pm-debug.c            |   17 +++++--
 arch/arm/mach-omap2/pm.h                  |   28 ++++++++++--
 arch/arm/mach-omap2/pm44xx.c              |   45 +++++++++++++++++++
 arch/arm/mach-omap2/prm44xx.c             |   66 +++++++++++++++++++++++++++++
 6 files changed, 214 insertions(+), 13 deletions(-)

arch/arm/mach-omap2/omap-mpuss-lowpower.c +2 -0

index e02c082..b9a2cc7 100644
@@ -60,6 +60,7 @@
 #include "prcm44xx.h"
 #include "prm44xx.h"
 #include "prm-regbits-44xx.h"
+#include "cm44xx.h"
 
 #ifdef CONFIG_SMP
 

@@ -232,6 +233,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 {
 	unsigned int save_state = 0;
 	unsigned int wakeup_cpu;
+	int ret;
 
 	if (omap_rev() == OMAP4430_REV_ES1_0)
 		return -ENXIO;

Hi Tero,

Quoted 48 lines · Tero Kristo

This patch adds device off support to OMAP4 device type.

OFF mode is disabled by default, however, there are two ways to enable OFF mode:
a) In the board file, call omap4_pm_off_mode_enable(1)
b) Enable OFF mode using the debugfs entry
echo "1">/sys/kernel/debug/pm_debug/enable_off_mode (conversely echo '0' will disable it as well).

Signed-off-by Santosh Shilimkar <[email protected]>

[t-kristo at ti.com: largely re-structured the code]

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |   24 ++++++++++-
 arch/arm/mach-omap2/omap-wakeupgen.c      |   47 +++++++++++++++++++-
 arch/arm/mach-omap2/pm-debug.c            |   17 +++++--
 arch/arm/mach-omap2/pm.h                  |   28 ++++++++++--
 arch/arm/mach-omap2/pm44xx.c              |   45 +++++++++++++++++++
 arch/arm/mach-omap2/prm44xx.c             |   66 +++++++++++++++++++++++++++++
 6 files changed, 214 insertions(+), 13 deletions(-)

arch/arm/mach-omap2/omap-mpuss-lowpower.c +3 -2

index e02c082..b9a2cc7 100644
@@ -60,6 +60,7 @@
 #include "prcm44xx.h"
 #include "prm44xx.h"
 #include "prm-regbits-44xx.h"
+#include "cm44xx.h"
 
 #ifdef CONFIG_SMP
 

@@ -232,6 +233,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 {
 	unsigned int save_state = 0;
 	unsigned int wakeup_cpu;
+	int ret;
 
 	if (omap_rev() == OMAP4430_REV_ES1_0)
 		return -ENXIO;

@@ -263,9 +265,21 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 	 * In MPUSS OSWR or device OFF, interrupt controller  contest is lost.
 	 */
 	mpuss_clear_prev_logic_pwrst();
-	if ((pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_RET) &&
-		(pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF))
+	if (omap4_device_next_state_off()) {

arch/arm/mach-omap2/omap-mpuss-lowpower.c int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) · line 272

Quoted 4 lines

arch/arm/mach-omap2/omap-mpuss-lowpower.c +4 -0

+		/* Save the device context to SAR RAM */
+		ret = omap_sar_save();
+		if (ret)
+			return ret;

Is it safe to simply return here? I was not sure if we need to call pwrdm_post_transition, given that we have already called pwrdm_pre_transition on entry.

From: Santosh Shilimkar <[email protected]>

The ROM BUG is when MPU Domain OFF wake up sequence that can compromise IVA and Tesla execution.

At wakeup from MPU OFF on HS device only (not GP device), when restoring the Secure RAM, the ROM Code reconfigures the clocks the same way it is done at Cold Reset.
The IVAHD Clocks and Power Domain settings are:
IVAHD_CM2 IVAHD_CLKCTRL_MODULE_MODE = DISABLE IVAHD_CM2 SL2_CLKCTRL_MODULE_MODE = DISABLE IVAHD_CM2 SL2_CLKSTCTRL_CLKTRCTRL = HW_AUTO IVAHD_PRM IVAHD_PWRSTCTRL_POWERSTATE = OFF
The TESLA Clocks and Power Domain settings are:
TESLA_CM1 TESLA_CLKCTRL_MODULE_MODE = DISABLE TESLA_CM1 TESLA_CLKSTCTRL_CLKTRCTRL = HW_AUTO TESLA_PRM TESLA_PWRSTCTRL_POWERSTATE = OFF

This patch fixes the low power OFF mode code so that the these registers are saved and restore across MPU OFF state.

Also because of this limitation, MPU OFF alone is not targeted without device OFF to avoid IVAHD and TESLA execution impact

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

[t-kristo at ti.com: added omap4 pm errata support]

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |   53 +++++++++++++++++++++++++++++
 arch/arm/mach-omap2/pm.h                  |    2 +
 arch/arm/mach-omap2/pm44xx.c              |    9 +++++
 3 files changed, 64 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/omap-mpuss-lowpower.c +53 -0

index b9a2cc7..208d4a4 100644
@@ -52,6 +52,7 @@
 
 #include <plat/omap44xx.h>
 
+#include "iomap.h"
 #include "common.h"
 #include "omap4-sar-layout.h"
 #include "pm.h"

@@ -76,6 +77,24 @@ static DEFINE_PER_CPU(struct omap4_cpu_pm_info, omap4_pm_info);
 static struct powerdomain *mpuss_pd;
 static void __iomem *sar_base;
 
+struct reg_tuple {
+	void __iomem *addr;
+	u32 val;
+};
+
+static struct reg_tuple tesla_reg[] = {
+	{.addr = OMAP4430_CM_TESLA_CLKSTCTRL},
+	{.addr = OMAP4430_CM_TESLA_TESLA_CLKCTRL},
+	{.addr = OMAP4430_PM_TESLA_PWRSTCTRL},
+};
+
+static struct reg_tuple ivahd_reg[] = {
+	{.addr = OMAP4430_CM_IVAHD_CLKSTCTRL},
+	{.addr = OMAP4430_CM_IVAHD_IVAHD_CLKCTRL},
+	{.addr = OMAP4430_CM_IVAHD_SL2_CLKCTRL},
+	{.addr = OMAP4430_PM_IVAHD_PWRSTCTRL}
+};
+
 /*
  * Program the wakeup routine address for the CPU0 and CPU1
  * used for OFF or DORMANT wakeup.

@@ -215,6 +234,34 @@ static void save_l2x0_context(void)
 {}
 #endif
 
+static inline void save_ivahd_tesla_regs(void)
+{
+	int i;
+
+	if (!IS_PM44XX_ERRATUM(PM_OMAP4_ROM_IVAHD_TESLA_ERRATUM))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(tesla_reg); i++)
+		tesla_reg[i].val = __raw_readl(tesla_reg[i].addr);
+
+	for (i = 0; i < ARRAY_SIZE(ivahd_reg); i++)
+		ivahd_reg[i].val = __raw_readl(ivahd_reg[i].addr);
+}
+
+static inline void restore_ivahd_tesla_regs(void)
+{
+	int i;
+
+	if (!IS_PM44XX_ERRATUM(PM_OMAP4_ROM_IVAHD_TESLA_ERRATUM))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(tesla_reg); i++)
+		__raw_writel(tesla_reg[i].val, tesla_reg[i].addr);
+
+	for (i = 0; i < ARRAY_SIZE(ivahd_reg); i++)
+		__raw_writel(ivahd_reg[i].val, ivahd_reg[i].addr);
+}
+
 /**
  * omap4_enter_lowpower: OMAP4 MPUSS Low Power Entry Function
  * The purpose of this function is to manage low power programming

@@ -273,11 +320,14 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 		omap_sar_overwrite();
 		omap4_cm_prepare_off();
 		omap4_dpll_prepare_off();
+		save_ivahd_tesla_regs();
 		save_state = 3;
 	} else if ((pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_RET) &&
 		(pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF)) {
+		save_ivahd_tesla_regs();
 		save_state = 2;
 	} else if (pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_OFF) {
+		save_ivahd_tesla_regs();
 		save_state = 3;
 	}
 

@@ -302,6 +352,9 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 	wakeup_cpu = smp_processor_id();
 	set_cpu_next_pwrst(wakeup_cpu, PWRDM_POWER_ON);
 
+	if (omap4_mpuss_read_prev_context_state())
+		restore_ivahd_tesla_regs();
+
 	if (omap4_device_prev_state_off()) {
 		omap4_dpll_resume_off();
 		omap4_cm_resume_off();

arch/arm/mach-omap2/pm.h +2 -0

index e78ec63..051aeb9 100644
@@ -131,6 +131,8 @@ extern void enable_omap3630_toggle_l2_on_restore(void);
 static inline void enable_omap3630_toggle_l2_on_restore(void) { }
 #endif		/* defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3) */
 
+#define PM_OMAP4_ROM_IVAHD_TESLA_ERRATUM	(1 << 0)
+
 #if defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP4)
 extern u16 pm44xx_errata;
 #define IS_PM44XX_ERRATUM(id)		(pm44xx_errata & (id))

arch/arm/mach-omap2/pm44xx.c +0 -0

index 8238097..67cb799 100644
@@ -263,6 +263,15 @@ static int __init omap4_pm_init(void)
 

Hi Tero,

Quoted 18 lines · Tero Kristo

From: Santosh Shilimkar <[email protected]>

The ROM BUG is when MPU Domain OFF wake up sequence that can compromise IVA and Tesla execution.

At wakeup from MPU OFF on HS device only (not GP device), when restoring the Secure RAM, the ROM Code reconfigures the clocks the same way it is done at Cold Reset.
The IVAHD Clocks and Power Domain settings are:
IVAHD_CM2 IVAHD_CLKCTRL_MODULE_MODE = DISABLE IVAHD_CM2 SL2_CLKCTRL_MODULE_MODE = DISABLE IVAHD_CM2 SL2_CLKSTCTRL_CLKTRCTRL = HW_AUTO IVAHD_PRM IVAHD_PWRSTCTRL_POWERSTATE = OFF
The TESLA Clocks and Power Domain settings are:
TESLA_CM1 TESLA_CLKCTRL_MODULE_MODE = DISABLE TESLA_CM1 TESLA_CLKSTCTRL_CLKTRCTRL = HW_AUTO TESLA_PRM TESLA_PWRSTCTRL_POWERSTATE = OFF

Quoted 5 lines

This patch fixes the low power OFF mode code so that the these registers are saved and restore across MPU OFF state.

Also because of this limitation, MPU OFF alone is not targeted without device OFF to avoid IVAHD and TESLA execution impact

You may wish to state which devices is impacted by this in the changelog.

Quoted 129 lines

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

[t-kristo at ti.com: added omap4 pm errata support]

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |   53 +++++++++++++++++++++++++++++
 arch/arm/mach-omap2/pm.h                  |    2 +
 arch/arm/mach-omap2/pm44xx.c              |    9 +++++
 3 files changed, 64 insertions(+), 0 deletions(-)

arch/arm/mach-omap2/omap-mpuss-lowpower.c +53 -0

index b9a2cc7..208d4a4 100644
@@ -52,6 +52,7 @@
 
 #include <plat/omap44xx.h>
 
+#include "iomap.h"
 #include "common.h"
 #include "omap4-sar-layout.h"
 #include "pm.h"

@@ -76,6 +77,24 @@ static DEFINE_PER_CPU(struct omap4_cpu_pm_info, omap4_pm_info);
 static struct powerdomain *mpuss_pd;
 static void __iomem *sar_base;
 
+struct reg_tuple {
+	void __iomem *addr;
+	u32 val;
+};
+
+static struct reg_tuple tesla_reg[] = {
+	{.addr = OMAP4430_CM_TESLA_CLKSTCTRL},
+	{.addr = OMAP4430_CM_TESLA_TESLA_CLKCTRL},
+	{.addr = OMAP4430_PM_TESLA_PWRSTCTRL},
+};
+
+static struct reg_tuple ivahd_reg[] = {
+	{.addr = OMAP4430_CM_IVAHD_CLKSTCTRL},
+	{.addr = OMAP4430_CM_IVAHD_IVAHD_CLKCTRL},
+	{.addr = OMAP4430_CM_IVAHD_SL2_CLKCTRL},
+	{.addr = OMAP4430_PM_IVAHD_PWRSTCTRL}
+};
+
 /*
  * Program the wakeup routine address for the CPU0 and CPU1
  * used for OFF or DORMANT wakeup.

@@ -215,6 +234,34 @@ static void save_l2x0_context(void)
 {}
 #endif
 
+static inline void save_ivahd_tesla_regs(void)
+{
+	int i;
+
+	if (!IS_PM44XX_ERRATUM(PM_OMAP4_ROM_IVAHD_TESLA_ERRATUM))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(tesla_reg); i++)
+		tesla_reg[i].val = __raw_readl(tesla_reg[i].addr);
+
+	for (i = 0; i < ARRAY_SIZE(ivahd_reg); i++)
+		ivahd_reg[i].val = __raw_readl(ivahd_reg[i].addr);
+}
+
+static inline void restore_ivahd_tesla_regs(void)
+{
+	int i;
+
+	if (!IS_PM44XX_ERRATUM(PM_OMAP4_ROM_IVAHD_TESLA_ERRATUM))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(tesla_reg); i++)
+		__raw_writel(tesla_reg[i].val, tesla_reg[i].addr);
+
+	for (i = 0; i < ARRAY_SIZE(ivahd_reg); i++)
+		__raw_writel(ivahd_reg[i].val, ivahd_reg[i].addr);
+}
+
 /**
  * omap4_enter_lowpower: OMAP4 MPUSS Low Power Entry Function
  * The purpose of this function is to manage low power programming

@@ -273,11 +320,14 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 		omap_sar_overwrite();
 		omap4_cm_prepare_off();
 		omap4_dpll_prepare_off();
+		save_ivahd_tesla_regs();
 		save_state = 3;
 	} else if ((pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_RET) &&
 		(pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF)) {
+		save_ivahd_tesla_regs();
 		save_state = 2;
 	} else if (pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_OFF) {
+		save_ivahd_tesla_regs();
 		save_state = 3;
 	}
 

@@ -302,6 +352,9 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 	wakeup_cpu = smp_processor_id();
 	set_cpu_next_pwrst(wakeup_cpu, PWRDM_POWER_ON);
 
+	if (omap4_mpuss_read_prev_context_state())
+		restore_ivahd_tesla_regs();
+
 	if (omap4_device_prev_state_off()) {
 		omap4_dpll_resume_off();
 		omap4_cm_resume_off();

arch/arm/mach-omap2/pm.h +2 -0

index e78ec63..051aeb9 100644
@@ -131,6 +131,8 @@ extern void enable_omap3630_toggle_l2_on_restore(void);
 static inline void enable_omap3630_toggle_l2_on_restore(void) { }
 #endif		/* defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3) */
 
+#define PM_OMAP4_ROM_IVAHD_TESLA_ERRATUM	(1 << 0)
+
 #if defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP4)
 extern u16 pm44xx_errata;
 #define IS_PM44XX_ERRATUM(id)		(pm44xx_errata & (id))

arch/arm/mach-omap2/pm44xx.c +0 -0

index 8238097..67cb799 100644
@@ -263,6 +263,15 @@ static int __init omap4_pm_init(void)
 
 	(void) clkdm_for_each(omap_pm_clkdms_setup, NULL);