[PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Reddy, Teerth]

From: Teerth Reddy <teerth@ti.com>

This patch sets the dpll3 clock stabilization delay during
DVFS. The stabilization delay is calculated dynamically 
using the ARM performance counter.
Currently 6us of SDRC stabilization value is used to get 
the correct delay.

Signed-off-by: Romit Dasgupta <romit@ti.com>
Signed-off-by: Teerth Reddy <teerth@ti.com>
---
 arch/arm/mach-omap2/clock34xx.c        |   32 ++++++++++------------
 arch/arm/mach-omap2/sram34xx.S         |    7 ++++
 arch/arm/plat-omap/Makefile            |    1 
 arch/arm/plat-omap/include/plat/pmc.h  |    9 ++++++
 arch/arm/plat-omap/include/plat/sram.h |    6 ++++
 arch/arm/plat-omap/pmc.c               |   47 +++++++++++++++++++++++++++++++++
 arch/arm/plat-omap/sram.c              |   23 ++++++++++++++++
 7 files changed, 108 insertions(+), 17 deletions(-)

Index: linux-omap-pm/arch/arm/plat-omap/Makefile
===================================================================
--- linux-omap-pm.orig/arch/arm/plat-omap/Makefile	2009-12-08 20:12:15.000000000 +0530
+++ linux-omap-pm/arch/arm/plat-omap/Makefile	2009-12-09 11:30:58.000000000 +0530
@@ -15,6 +15,7 @@
 # omap_device support (OMAP2+ only at the moment)
 obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
 obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
+obj-$(CONFIG_ARCH_OMAP3) += pmc.o
 
 obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
 obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
Index: linux-omap-pm/arch/arm/plat-omap/include/plat/pmc.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-omap-pm/arch/arm/plat-omap/include/plat/pmc.h	2009-12-09 11:30:58.000000000 +0530
@@ -0,0 +1,9 @@
+#ifndef __PMC_H__
+#define __PMC_H__
+extern void start_perf_counter(void);
+extern void start_cycle_counter(void);
+extern void stop_cycle_counter(void);
+extern void stop_perf_counter(void);
+extern unsigned int cycle_count(void);
+extern unsigned int delay_sram;
+#endif
Index: linux-omap-pm/arch/arm/plat-omap/include/plat/sram.h
===================================================================
--- linux-omap-pm.orig/arch/arm/plat-omap/include/plat/sram.h	2009-12-08 20:12:15.000000000 +0530
+++ linux-omap-pm/arch/arm/plat-omap/include/plat/sram.h	2009-12-09 11:30:58.000000000 +0530
@@ -69,6 +69,12 @@
 			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
 extern unsigned long omap3_sram_configure_core_dpll_sz;
 
+extern unsigned int measure_sram_delay(unsigned int);
+
+extern u32 __sram_wait_delay(
+		unsigned int);
+extern unsigned long __sram_wait_delay_sz;
+
 #ifdef CONFIG_PM
 extern void omap_push_sram_idle(void);
 #else
Index: linux-omap-pm/arch/arm/plat-omap/pmc.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-omap-pm/arch/arm/plat-omap/pmc.c	2009-12-09 11:30:58.000000000 +0530
@@ -0,0 +1,47 @@
+#include <linux/module.h>
+
+void start_perf_counter(void)
+{
+	unsigned int r1 = 0;
+	asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
+	r1 |= 0x1; /* enable counters */
+	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
+	return;
+}
+EXPORT_SYMBOL(start_perf_counter);
+
+void start_cycle_counter(void)
+{
+	unsigned int r2 = 0;
+	r2 = 0x80000000; /* enable cycle counter only */
+	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r2));
+	return;
+}
+EXPORT_SYMBOL(start_cycle_counter);
+
+unsigned int cycle_count(void)
+{
+	unsigned int rd = 0;
+	asm("mrc p15, 0, %0, c9, c13, 0" : "=r" (rd));
+	return rd;
+}
+EXPORT_SYMBOL(cycle_count);
+
+
+void stop_cycle_counter(void)
+{
+	unsigned int r3 = 0;
+	r3 = 0x0; /* disable cycle counter */
+	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r3));
+	return;
+}
+EXPORT_SYMBOL(stop_cycle_counter);
+
+void stop_perf_counter(void)
+{
+	unsigned int r1 = 0;
+	r1 = 0x0; /* disable counters */
+	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
+	return;
+}
+EXPORT_SYMBOL(stop_perf_counter);
Index: linux-omap-pm/arch/arm/plat-omap/sram.c
===================================================================
--- linux-omap-pm.orig/arch/arm/plat-omap/sram.c	2009-12-08 20:12:15.000000000 +0530
+++ linux-omap-pm/arch/arm/plat-omap/sram.c	2009-12-09 14:00:56.000000000 +0530
@@ -29,6 +29,10 @@
 #include <plat/board.h>
 #include <plat/cpu.h>
 
+#ifdef CONFIG_ARCH_OMAP3
+#include <plat/pmc.h>
+#endif
+
 #include <plat/control.h>
 
 #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
@@ -423,6 +427,25 @@
 }
 #endif
 
+
+#ifdef CONFIG_ARCH_OMAP3
+unsigned int measure_sram_delay(unsigned int loop)
+{
+	unsigned int start = 0, end = 0;
+	void (*_omap3_sram_delay)(unsigned long);
+	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
+						__sram_wait_delay_sz);
+	start_perf_counter();
+	start_cycle_counter();
+	start = cycle_count();
+	_omap3_sram_delay(loop);
+	end = cycle_count();
+	stop_cycle_counter();
+	stop_perf_counter();
+	return end - start;
+}
+#endif
+
 int __init omap_sram_init(void)
 {
 	omap_detect_sram();
Index: linux-omap-pm/arch/arm/mach-omap2/clock34xx.c
===================================================================
--- linux-omap-pm.orig/arch/arm/mach-omap2/clock34xx.c	2009-12-08 20:12:15.000000000 +0530
+++ linux-omap-pm/arch/arm/mach-omap2/clock34xx.c	2009-12-11 14:38:38.000000000 +0530
@@ -37,6 +37,8 @@
 #include <asm/div64.h>
 #include <asm/clkdev.h>
 
+#include <plat/pmc.h>
+
 #include <plat/sdrc.h>
 #include "clock.h"
 #include "prm.h"
@@ -46,6 +48,8 @@
 
 static const struct clkops clkops_noncore_dpll_ops;
 
+unsigned int delay_sram;
+
 static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
 					    void __iomem **idlest_reg,
 					    u8 *idlest_bit);
@@ -333,14 +337,9 @@
 /* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
 #define SDRC_MPURATE_SCALE		8
 
-/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined for */
-#define SDRC_MPURATE_BASE_SHIFT		9
-
-/*
- * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
- * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
- */
-#define SDRC_MPURATE_LOOPS		96
+/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
+/* hardcode currently */
+#define	SDRC_TIME_STABILIZE		6
 
 /*
  * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
@@ -870,16 +869,10 @@
 		unlock_dll = 1;
 	}
 
-	/*
-	 * XXX This only needs to be done when the CPU frequency changes
-	 */
+	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
+
 	mpurate = arm_fck.rate / CYCLES_PER_MHZ;
-	c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
-	c += 1;  /* for safety */
-	c *= SDRC_MPURATE_LOOPS;
-	c >>= SDRC_MPURATE_SCALE;
-	if (c == 0)
-		c = 1;
+	c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
 
 	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
 		 validrate);
@@ -1228,6 +1221,11 @@
 	vclk = clk_get(NULL, "virt_prcm_set");
 	sclk = clk_get(NULL, "sys_ck");
 #endif
+
+	/* Measure sram delay */
+	delay_sram = measure_sram_delay(10000)/(10000*2);
+	pr_debug("SRAM delay: %d\n", delay_sram);
+
 	return 0;
 }
 
Index: linux-omap-pm/arch/arm/mach-omap2/sram34xx.S
===================================================================
--- linux-omap-pm.orig/arch/arm/mach-omap2/sram34xx.S	2009-12-08 20:12:15.000000000 +0530
+++ linux-omap-pm/arch/arm/mach-omap2/sram34xx.S	2009-12-09 11:30:58.000000000 +0530
@@ -298,3 +298,10 @@
 ENTRY(omap3_sram_configure_core_dpll_sz)
 	.word	. - omap3_sram_configure_core_dpll
 
+ENTRY(__sram_wait_delay)
+	subs 	r0, r0, #1
+	bne	__sram_wait_delay
+	bx	lr
+
+ENTRY(__sram_wait_delay_sz)
+	.word	. - __sram_wait_delay

[PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Reddy, Teerth]

Reposting the patch with proper format

From: Teerth Reddy <teerth@ti.com>

This patch sets the dpll3 clock stabilization delay during
DVFS. The stabilization delay is calculated dynamically 
using the ARM performance counter.
Currently 6us of SDRC stabilization value is used to get 
the correct delay.

Signed-off-by: Romit Dasgupta <romit@ti.com>
Signed-off-by: Teerth Reddy <teerth@ti.com>
---
 mach-omap2/clock34xx.c        |   32 +++++++++++++---------------
 mach-omap2/sram34xx.S         |    7 ++++++
 plat-omap/Makefile            |    1
 plat-omap/include/plat/pmc.h  |    9 ++++++++
 plat-omap/include/plat/sram.h |    6 +++++
 plat-omap/pmc.c               |   47 ++++++++++++++++++++++++++++++++++++++++++
 plat-omap/sram.c              |   23 ++++++++++++++++++++
 7 files changed, 108 insertions(+), 17 deletions(-)


diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index da5bc1f..dd49938 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -37,6 +37,8 @@
 #include <asm/div64.h>
 #include <asm/clkdev.h>
 
+#include <plat/pmc.h>
+
 #include <plat/sdrc.h>
 #include "clock.h"
 #include "prm.h"
@@ -46,6 +48,8 @@
 
 static const struct clkops clkops_noncore_dpll_ops;
 
+unsigned int delay_sram;
+
 static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
 					    void __iomem **idlest_reg,
 					    u8 *idlest_bit);
@@ -333,14 +337,9 @@ static struct omap_clk omap34xx_clks[] = {
 /* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
 #define SDRC_MPURATE_SCALE		8
 
-/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined for */
-#define SDRC_MPURATE_BASE_SHIFT		9
-
-/*
- * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
- * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
- */
-#define SDRC_MPURATE_LOOPS		96
+/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
+/* hardcoded currently */
+#define	SDRC_TIME_STABILIZE		6
 
 /*
  * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
@@ -870,16 +869,10 @@ static int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
 		unlock_dll = 1;
 	}
 
-	/*
-	 * XXX This only needs to be done when the CPU frequency changes
-	 */
+	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
+
 	mpurate = arm_fck.rate / CYCLES_PER_MHZ;
-	c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
-	c += 1;  /* for safety */
-	c *= SDRC_MPURATE_LOOPS;
-	c >>= SDRC_MPURATE_SCALE;
-	if (c == 0)
-		c = 1;
+	c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
 
 	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
 		 validrate);
@@ -1228,6 +1221,11 @@ int __init omap2_clk_init(void)
 	vclk = clk_get(NULL, "virt_prcm_set");
 	sclk = clk_get(NULL, "sys_ck");
 #endif
+
+	/* Measure sram delay */
+	delay_sram = measure_sram_delay(10000)/(10000*2);
+	pr_debug("SRAM delay: %d\n", delay_sram);
+
 	return 0;
 }
 
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 82aa4a3..dc0ac72 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -298,3 +298,10 @@ core_m2_mask_val:
 ENTRY(omap3_sram_configure_core_dpll_sz)
 	.word	. - omap3_sram_configure_core_dpll
 
+ENTRY(__sram_wait_delay)
+	subs 	r0, r0, #1
+	bne	__sram_wait_delay
+	bx	lr
+
+ENTRY(__sram_wait_delay_sz)
+	.word	. - __sram_wait_delay
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 95f8413..aac43da 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
 # omap_device support (OMAP2+ only at the moment)
 obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
 obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
+obj-$(CONFIG_ARCH_OMAP3) += pmc.o
 
 obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
 obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
diff --git a/arch/arm/plat-omap/include/plat/pmc.h b/arch/arm/plat-omap/include/plat/pmc.h
new file mode 100644
index 0000000..6d73782
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/pmc.h
@@ -0,0 +1,9 @@
+#ifndef __PMC_H__
+#define __PMC_H__
+extern void start_perf_counter(void);
+extern void start_cycle_counter(void);
+extern void stop_cycle_counter(void);
+extern void stop_perf_counter(void);
+extern unsigned int cycle_count(void);
+extern unsigned int delay_sram;
+#endif
diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
index 16a1b45..6019ed8 100644
--- a/arch/arm/plat-omap/include/plat/sram.h
+++ b/arch/arm/plat-omap/include/plat/sram.h
@@ -69,6 +69,12 @@ extern u32 omap3_sram_configure_core_dpll(
 			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
 extern unsigned long omap3_sram_configure_core_dpll_sz;
 
+extern unsigned int measure_sram_delay(unsigned int);
+
+extern u32 __sram_wait_delay(
+		unsigned int);
+extern unsigned long __sram_wait_delay_sz;
+
 #ifdef CONFIG_PM
 extern void omap_push_sram_idle(void);
 #else
diff --git a/arch/arm/plat-omap/pmc.c b/arch/arm/plat-omap/pmc.c
new file mode 100644
index 0000000..11acf42
--- /dev/null
+++ b/arch/arm/plat-omap/pmc.c
@@ -0,0 +1,47 @@
+#include <linux/module.h>
+
+void start_perf_counter(void)
+{
+	unsigned int r1 = 0;
+	asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
+	r1 |= 0x1; /* enable counters */
+	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
+	return;
+}
+EXPORT_SYMBOL(start_perf_counter);
+
+void start_cycle_counter(void)
+{
+	unsigned int r2 = 0;
+	r2 = 0x80000000; /* enable cycle counter only */
+	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r2));
+	return;
+}
+EXPORT_SYMBOL(start_cycle_counter);
+
+unsigned int cycle_count(void)
+{
+	unsigned int rd = 0;
+	asm("mrc p15, 0, %0, c9, c13, 0" : "=r" (rd));
+	return rd;
+}
+EXPORT_SYMBOL(cycle_count);
+
+
+void stop_cycle_counter(void)
+{
+	unsigned int r3 = 0;
+	r3 = 0x0; /* disable cycle counter */
+	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r3));
+	return;
+}
+EXPORT_SYMBOL(stop_cycle_counter);
+
+void stop_perf_counter(void)
+{
+	unsigned int r1 = 0;
+	r1 = 0x0; /* disable counters */
+	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
+	return;
+}
+EXPORT_SYMBOL(stop_perf_counter);
diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
index 3e92366..4ed6a48 100644
--- a/arch/arm/plat-omap/sram.c
+++ b/arch/arm/plat-omap/sram.c
@@ -29,6 +29,10 @@
 #include <plat/board.h>
 #include <plat/cpu.h>
 
+#ifdef CONFIG_ARCH_OMAP3
+#include <plat/pmc.h>
+#endif
+
 #include <plat/control.h>
 
 #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
@@ -423,6 +427,25 @@ static inline int omap34xx_sram_init(void)
 }
 #endif
 
+
+#ifdef CONFIG_ARCH_OMAP3
+unsigned int measure_sram_delay(unsigned int loop)
+{
+	unsigned int start = 0, end = 0;
+	void (*_omap3_sram_delay)(unsigned long);
+	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
+						__sram_wait_delay_sz);
+	start_perf_counter();
+	start_cycle_counter();
+	start = cycle_count();
+	_omap3_sram_delay(loop);
+	end = cycle_count();
+	stop_cycle_counter();
+	stop_perf_counter();
+	return end - start;
+}
+#endif
+
 int __init omap_sram_init(void)
 {
 	omap_detect_sram();

RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Menon, Nishanth]

> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Reddy, Teerth
> Sent: Friday, December 11, 2009 6:06 AM
> To: linux-omap@vger.kernel.org
> Subject: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock
> stabilization delay
> 
> Reposting the patch with proper format

Thanks for doing something automated instead of the old black magic
Loops based on SDRC_MPURATE_BASE_SHIFT tuning :) (hip hip hoorah - no
More days figuring out why display behaves weird when vdd2 opp switched
;) ).. anyways, my comments follow..

> 
> From: Teerth Reddy <teerth@ti.com>
> 
> This patch sets the dpll3 clock stabilization delay during
> DVFS. The stabilization delay is calculated dynamically
> using the ARM performance counter.
> Currently 6us of SDRC stabilization value is used to get
> the correct delay.

Is this hard 6uS delay valid for all SDRAMs and board variants? If your
Claim for dynamic detection is true, you don't need be using hard values
This is the type of #def mess we got out of MPURATE_BASE_SHIFT in the first
place..

Also Richard indicated that there might be a few tricky things with perf
Counters with specific devices - like EMU/HS/GP devices. It might need EMU
Domain for the values to pass through and there might be a yet-not-measured increase In power which could impact usage numbers and may need additional 
Code to switch off the domain correctly while hitting OFF/RET..

Might be good if community can try this on various OMAP3s and various SDRAMs
Mebbe who ever has access, could even try on EMU/HS/ etc..

> 
> Signed-off-by: Romit Dasgupta <romit@ti.com>
> Signed-off-by: Teerth Reddy <teerth@ti.com>
> ---
>  mach-omap2/clock34xx.c        |   32 +++++++++++++---------------
>  mach-omap2/sram34xx.S         |    7 ++++++
>  plat-omap/Makefile            |    1
>  plat-omap/include/plat/pmc.h  |    9 ++++++++
>  plat-omap/include/plat/sram.h |    6 +++++
>  plat-omap/pmc.c               |   47
> ++++++++++++++++++++++++++++++++++++++++++
>  plat-omap/sram.c              |   23 ++++++++++++++++++++
>  7 files changed, 108 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-
> omap2/clock34xx.c
> index da5bc1f..dd49938 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -37,6 +37,8 @@
>  #include <asm/div64.h>
>  #include <asm/clkdev.h>
> 
> +#include <plat/pmc.h>
> +
>  #include <plat/sdrc.h>
>  #include "clock.h"
>  #include "prm.h"
> @@ -46,6 +48,8 @@
> 
>  static const struct clkops clkops_noncore_dpll_ops;
> 
> +unsigned int delay_sram;
> +
>  static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
>  					    void __iomem **idlest_reg,
>  					    u8 *idlest_bit);
> @@ -333,14 +337,9 @@ static struct omap_clk omap34xx_clks[] = {
>  /* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
>  #define SDRC_MPURATE_SCALE		8
> 
> -/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined
> for */
> -#define SDRC_MPURATE_BASE_SHIFT		9
> -
> -/*
> - * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
> - * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
> - */
> -#define SDRC_MPURATE_LOOPS		96
> +/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
> +/* hardcoded currently */
> +#define	SDRC_TIME_STABILIZE		6

Need a reason why we are hard coding -> it will be good to know if this
Delay varies across boards, and if so, you may need this info to come from
 board files OR derived out of sdrc parameters already being provided by 
board files (this would be my preferred way though).


> 
>  /*
>   * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
> @@ -870,16 +869,10 @@ static int omap3_core_dpll_m2_set_rate(struct clk
> *clk, unsigned long rate)
>  		unlock_dll = 1;
>  	}
> 
> -	/*
> -	 * XXX This only needs to be done when the CPU frequency changes
> -	 */
> +	/* Calculate the number of MPU cycles to wait for SDRC to stabilize
> */
> +
>  	mpurate = arm_fck.rate / CYCLES_PER_MHZ;
> -	c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
> -	c += 1;  /* for safety */
> -	c *= SDRC_MPURATE_LOOPS;
> -	c >>= SDRC_MPURATE_SCALE;
> -	if (c == 0)
> -		c = 1;
> +	c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
> 
>  	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk-
> >rate,
>  		 validrate);
> @@ -1228,6 +1221,11 @@ int __init omap2_clk_init(void)
>  	vclk = clk_get(NULL, "virt_prcm_set");
>  	sclk = clk_get(NULL, "sys_ck");
>  #endif
> +
> +	/* Measure sram delay */
> +	delay_sram = measure_sram_delay(10000)/(10000*2);
                                             ^^^^^^^^^ <- you have not run
 checkpatch.pl on this patch in the first place.. sorry.. that should be 
easy to do I guess..

> +	pr_debug("SRAM delay: %d\n", delay_sram);


Arrgh more spam..

> +
>  	return 0;
>  }
> 
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-
> omap2/sram34xx.S
> index 82aa4a3..dc0ac72 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -298,3 +298,10 @@ core_m2_mask_val:
>  ENTRY(omap3_sram_configure_core_dpll_sz)
>  	.word	. - omap3_sram_configure_core_dpll
> 
> +ENTRY(__sram_wait_delay)
> +	subs 	r0, r0, #1
> +	bne	__sram_wait_delay
> +	bx	lr
> +
> +ENTRY(__sram_wait_delay_sz)
> +	.word	. - __sram_wait_delay
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 95f8413..aac43da 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>  # omap_device support (OMAP2+ only at the moment)
>  obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
>  obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
> +obj-$(CONFIG_ARCH_OMAP3) += pmc.o
> 
>  obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
>  obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
> diff --git a/arch/arm/plat-omap/include/plat/pmc.h b/arch/arm/plat-
> omap/include/plat/pmc.h
> new file mode 100644
> index 0000000..6d73782
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/pmc.h
> @@ -0,0 +1,9 @@
> +#ifndef __PMC_H__
> +#define __PMC_H__
> +extern void start_perf_counter(void);
> +extern void start_cycle_counter(void);
> +extern void stop_cycle_counter(void);
> +extern void stop_perf_counter(void);
> +extern unsigned int cycle_count(void);
> +extern unsigned int delay_sram;
> +#endif
> diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-
> omap/include/plat/sram.h
> index 16a1b45..6019ed8 100644
> --- a/arch/arm/plat-omap/include/plat/sram.h
> +++ b/arch/arm/plat-omap/include/plat/sram.h
> @@ -69,6 +69,12 @@ extern u32 omap3_sram_configure_core_dpll(
>  			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
>  extern unsigned long omap3_sram_configure_core_dpll_sz;
> 
> +extern unsigned int measure_sram_delay(unsigned int);
> +
> +extern u32 __sram_wait_delay(
> +		unsigned int);

fix formatting please..

> +extern unsigned long __sram_wait_delay_sz;
> +
>  #ifdef CONFIG_PM
>  extern void omap_push_sram_idle(void);
>  #else
> diff --git a/arch/arm/plat-omap/pmc.c b/arch/arm/plat-omap/pmc.c
> new file mode 100644
> index 0000000..11acf42
> --- /dev/null
> +++ b/arch/arm/plat-omap/pmc.c
> @@ -0,0 +1,47 @@
> +#include <linux/module.h>
> +
> +void start_perf_counter(void)
> +{
> +	unsigned int r1 = 0;
> +	asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
> +	r1 |= 0x1; /* enable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(start_perf_counter);
> +
> +void start_cycle_counter(void)
> +{
> +	unsigned int r2 = 0;
> +	r2 = 0x80000000; /* enable cycle counter only */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r2));
> +	return;
> +}
> +EXPORT_SYMBOL(start_cycle_counter);
> +
> +unsigned int cycle_count(void)
> +{
> +	unsigned int rd = 0;
> +	asm("mrc p15, 0, %0, c9, c13, 0" : "=r" (rd));
> +	return rd;
> +}
> +EXPORT_SYMBOL(cycle_count);
> +
> +
> +void stop_cycle_counter(void)
> +{
> +	unsigned int r3 = 0;
> +	r3 = 0x0; /* disable cycle counter */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r3));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_cycle_counter);
> +
> +void stop_perf_counter(void)
> +{
> +	unsigned int r1 = 0;
> +	r1 = 0x0; /* disable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_perf_counter);
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index 3e92366..4ed6a48 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -29,6 +29,10 @@
>  #include <plat/board.h>
>  #include <plat/cpu.h>
> 
> +#ifdef CONFIG_ARCH_OMAP3
> +#include <plat/pmc.h>
> +#endif
> +
>  #include <plat/control.h>
> 
>  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> @@ -423,6 +427,25 @@ static inline int omap34xx_sram_init(void)
>  }
>  #endif
> 
> +
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned int measure_sram_delay(unsigned int loop)
> +{
> +	unsigned int start = 0, end = 0;
> +	void (*_omap3_sram_delay)(unsigned long);
> +	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> +						__sram_wait_delay_sz);
> +	start_perf_counter();
> +	start_cycle_counter();
> +	start = cycle_count();
> +	_omap3_sram_delay(loop);
> +	end = cycle_count();
> +	stop_cycle_counter();
> +	stop_perf_counter();
> +	return end - start;
> +}
> +#endif
> +

>  int __init omap_sram_init(void)
>  {
>  	omap_detect_sram();
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Romit Dasgupta]

> 
> Also Richard indicated that there might be a few tricky things with perf
> Counters with specific devices - like EMU/HS/GP devices. It might need EMU
> Domain for the values to pass through and there might be a yet-not-measured increase In power which could impact usage numbers and may need additional 
> Code to switch off the domain correctly while hitting OFF/RET..
>

Yes someone with EMU/HS could run and let us know. OTOH there won't be any
increase in power as it is done only once during boot time after which the
perfcounters are stopped.

By the way can you run this in 3630 and help us find what is the SRAM access
delay? I am sure it should be lesser since it has a process improvement over 34xx.

[PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Romit Dasgupta]

Reposting on behalf of Teerth.




This patch sets the dpll3 clock stabilization delay during
DVFS. The stabilization delay is calculated dynamically 
using the ARM performance counter.
Currently 6us of SDRC stabilization value is used to get 
the correct delay.

Signed-off-by: Romit Dasgupta <romit@ti.com>
Signed-off-by: Teerth Reddy <teerth@ti.com>
---
 mach-omap2/clock34xx.c        |   32 +++++++++++++---------------
 mach-omap2/sram34xx.S         |    7 ++++++
 plat-omap/Makefile            |    1
 plat-omap/include/plat/pmc.h  |    9 ++++++++
 plat-omap/include/plat/sram.h |    6 +++++
 plat-omap/pmc.c               |   47 ++++++++++++++++++++++++++++++++++++++++++
 plat-omap/sram.c              |   23 ++++++++++++++++++++
 7 files changed, 108 insertions(+), 17 deletions(-)


diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index da5bc1f..dd49938 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -37,6 +37,8 @@
 #include <asm/div64.h>
 #include <asm/clkdev.h>
 
+#include <plat/pmc.h>
+
 #include <plat/sdrc.h>
 #include "clock.h"
 #include "prm.h"
@@ -46,6 +48,8 @@
 
 static const struct clkops clkops_noncore_dpll_ops;
 
+unsigned int delay_sram;
+
 static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
 					    void __iomem **idlest_reg,
 					    u8 *idlest_bit);
@@ -333,14 +337,9 @@ static struct omap_clk omap34xx_clks[] = {
 /* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
 #define SDRC_MPURATE_SCALE		8
 
-/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined for */
-#define SDRC_MPURATE_BASE_SHIFT		9
-
-/*
- * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
- * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
- */
-#define SDRC_MPURATE_LOOPS		96
+/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
+/* hardcoded currently */
+#define	SDRC_TIME_STABILIZE		6
 
 /*
  * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
@@ -870,16 +869,10 @@ static int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
 		unlock_dll = 1;
 	}
 
-	/*
-	 * XXX This only needs to be done when the CPU frequency changes
-	 */
+	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
+
 	mpurate = arm_fck.rate / CYCLES_PER_MHZ;
-	c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
-	c += 1;  /* for safety */
-	c *= SDRC_MPURATE_LOOPS;
-	c >>= SDRC_MPURATE_SCALE;
-	if (c == 0)
-		c = 1;
+	c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
 
 	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
 		 validrate);
@@ -1228,6 +1221,11 @@ int __init omap2_clk_init(void)
 	vclk = clk_get(NULL, "virt_prcm_set");
 	sclk = clk_get(NULL, "sys_ck");
 #endif
+
+	/* Measure sram delay */
+	delay_sram = measure_sram_delay(10000)/(10000*2);
+	pr_debug("SRAM delay: %d\n", delay_sram);
+
 	return 0;
 }
 
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 82aa4a3..dc0ac72 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -298,3 +298,10 @@ core_m2_mask_val:
 ENTRY(omap3_sram_configure_core_dpll_sz)
 	.word	. - omap3_sram_configure_core_dpll
 
+ENTRY(__sram_wait_delay)
+	subs 	r0, r0, #1
+	bne	__sram_wait_delay
+	bx	lr
+
+ENTRY(__sram_wait_delay_sz)
+	.word	. - __sram_wait_delay
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 95f8413..aac43da 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
 # omap_device support (OMAP2+ only at the moment)
 obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
 obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
+obj-$(CONFIG_ARCH_OMAP3) += pmc.o
 
 obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
 obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
diff --git a/arch/arm/plat-omap/include/plat/pmc.h b/arch/arm/plat-omap/include/plat/pmc.h
new file mode 100644
index 0000000..6d73782
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/pmc.h
@@ -0,0 +1,9 @@
+#ifndef __PMC_H__
+#define __PMC_H__
+extern void start_perf_counter(void);
+extern void start_cycle_counter(void);
+extern void stop_cycle_counter(void);
+extern void stop_perf_counter(void);
+extern unsigned int cycle_count(void);
+extern unsigned int delay_sram;
+#endif
diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
index 16a1b45..6019ed8 100644
--- a/arch/arm/plat-omap/include/plat/sram.h
+++ b/arch/arm/plat-omap/include/plat/sram.h
@@ -69,6 +69,12 @@ extern u32 omap3_sram_configure_core_dpll(
 			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
 extern unsigned long omap3_sram_configure_core_dpll_sz;
 
+extern unsigned int measure_sram_delay(unsigned int);
+
+extern u32 __sram_wait_delay(
+		unsigned int);
+extern unsigned long __sram_wait_delay_sz;
+
 #ifdef CONFIG_PM
 extern void omap_push_sram_idle(void);
 #else
diff --git a/arch/arm/plat-omap/pmc.c b/arch/arm/plat-omap/pmc.c
new file mode 100644
index 0000000..11acf42
--- /dev/null
+++ b/arch/arm/plat-omap/pmc.c
@@ -0,0 +1,47 @@
+#include <linux/module.h>
+
+void start_perf_counter(void)
+{
+	unsigned int r1 = 0;
+	asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
+	r1 |= 0x1; /* enable counters */
+	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
+	return;
+}
+EXPORT_SYMBOL(start_perf_counter);
+
+void start_cycle_counter(void)
+{
+	unsigned int r2 = 0;
+	r2 = 0x80000000; /* enable cycle counter only */
+	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r2));
+	return;
+}
+EXPORT_SYMBOL(start_cycle_counter);
+
+unsigned int cycle_count(void)
+{
+	unsigned int rd = 0;
+	asm("mrc p15, 0, %0, c9, c13, 0" : "=r" (rd));
+	return rd;
+}
+EXPORT_SYMBOL(cycle_count);
+
+
+void stop_cycle_counter(void)
+{
+	unsigned int r3 = 0;
+	r3 = 0x0; /* disable cycle counter */
+	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r3));
+	return;
+}
+EXPORT_SYMBOL(stop_cycle_counter);
+
+void stop_perf_counter(void)
+{
+	unsigned int r1 = 0;
+	r1 = 0x0; /* disable counters */
+	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
+	return;
+}
+EXPORT_SYMBOL(stop_perf_counter);
diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
index 3e92366..4ed6a48 100644
--- a/arch/arm/plat-omap/sram.c
+++ b/arch/arm/plat-omap/sram.c
@@ -29,6 +29,10 @@
 #include <plat/board.h>
 #include <plat/cpu.h>
 
+#ifdef CONFIG_ARCH_OMAP3
+#include <plat/pmc.h>
+#endif
+
 #include <plat/control.h>
 
 #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
@@ -423,6 +427,25 @@ static inline int omap34xx_sram_init(void)
 }
 #endif
 
+
+#ifdef CONFIG_ARCH_OMAP3
+unsigned int measure_sram_delay(unsigned int loop)
+{
+	unsigned int start = 0, end = 0;
+	void (*_omap3_sram_delay)(unsigned long);
+	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
+						__sram_wait_delay_sz);
+	start_perf_counter();
+	start_cycle_counter();
+	start = cycle_count();
+	_omap3_sram_delay(loop);
+	end = cycle_count();
+	stop_cycle_counter();
+	stop_perf_counter();
+	return end - start;
+}
+#endif
+
 int __init omap_sram_init(void)
 {
 	omap_detect_sram();

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Jean Pihet]

On Friday 11 December 2009 13:05:37 Reddy, Teerth wrote:
> Reposting the patch with proper format
>
> From: Teerth Reddy <teerth@ti.com>
>
> This patch sets the dpll3 clock stabilization delay during
> DVFS. The stabilization delay is calculated dynamically
> using the ARM performance counter.
> Currently 6us of SDRC stabilization value is used to get
> the correct delay.
That is a good thing to have! However the counters might already be in use, 
cf. below

>
> Signed-off-by: Romit Dasgupta <romit@ti.com>
> Signed-off-by: Teerth Reddy <teerth@ti.com>
> ---
>  mach-omap2/clock34xx.c        |   32 +++++++++++++---------------
>  mach-omap2/sram34xx.S         |    7 ++++++
>  plat-omap/Makefile            |    1
>  plat-omap/include/plat/pmc.h  |    9 ++++++++
>  plat-omap/include/plat/sram.h |    6 +++++
>  plat-omap/pmc.c               |   47
> ++++++++++++++++++++++++++++++++++++++++++ plat-omap/sram.c              | 
>  23 ++++++++++++++++++++
>  7 files changed, 108 insertions(+), 17 deletions(-)
>
>
> diff --git a/arch/arm/mach-omap2/clock34xx.c
> b/arch/arm/mach-omap2/clock34xx.c index da5bc1f..dd49938 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -37,6 +37,8 @@
>  #include <asm/div64.h>
>  #include <asm/clkdev.h>
>
> +#include <plat/pmc.h>
> +
>  #include <plat/sdrc.h>
>  #include "clock.h"
>  #include "prm.h"
> @@ -46,6 +48,8 @@
>
>  static const struct clkops clkops_noncore_dpll_ops;
>
> +unsigned int delay_sram;
> +
>  static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
>  					    void __iomem **idlest_reg,
>  					    u8 *idlest_bit);
> @@ -333,14 +337,9 @@ static struct omap_clk omap34xx_clks[] = {
>  /* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
>  #define SDRC_MPURATE_SCALE		8
>
> -/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined
> for */ -#define SDRC_MPURATE_BASE_SHIFT		9
> -
> -/*
> - * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
> - * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
> - */
> -#define SDRC_MPURATE_LOOPS		96
> +/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
> +/* hardcoded currently */
> +#define	SDRC_TIME_STABILIZE		6
>
>  /*
>   * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
> @@ -870,16 +869,10 @@ static int omap3_core_dpll_m2_set_rate(struct clk
> *clk, unsigned long rate) unlock_dll = 1;
>  	}
>
> -	/*
> -	 * XXX This only needs to be done when the CPU frequency changes
> -	 */
> +	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
> +
>  	mpurate = arm_fck.rate / CYCLES_PER_MHZ;
> -	c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
> -	c += 1;  /* for safety */
> -	c *= SDRC_MPURATE_LOOPS;
> -	c >>= SDRC_MPURATE_SCALE;
> -	if (c == 0)
> -		c = 1;
> +	c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
>
>  	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
>  		 validrate);
> @@ -1228,6 +1221,11 @@ int __init omap2_clk_init(void)
>  	vclk = clk_get(NULL, "virt_prcm_set");
>  	sclk = clk_get(NULL, "sys_ck");
>  #endif
> +
> +	/* Measure sram delay */
> +	delay_sram = measure_sram_delay(10000)/(10000*2);
> +	pr_debug("SRAM delay: %d\n", delay_sram);
> +
>  	return 0;
>  }
>
> diff --git a/arch/arm/mach-omap2/sram34xx.S
> b/arch/arm/mach-omap2/sram34xx.S index 82aa4a3..dc0ac72 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -298,3 +298,10 @@ core_m2_mask_val:
>  ENTRY(omap3_sram_configure_core_dpll_sz)
>  	.word	. - omap3_sram_configure_core_dpll
>
> +ENTRY(__sram_wait_delay)
> +	subs 	r0, r0, #1
> +	bne	__sram_wait_delay
> +	bx	lr
> +
> +ENTRY(__sram_wait_delay_sz)
> +	.word	. - __sram_wait_delay
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 95f8413..aac43da 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>  # omap_device support (OMAP2+ only at the moment)
>  obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
>  obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
> +obj-$(CONFIG_ARCH_OMAP3) += pmc.o
>
>  obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
>  obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
> diff --git a/arch/arm/plat-omap/include/plat/pmc.h
> b/arch/arm/plat-omap/include/plat/pmc.h new file mode 100644
> index 0000000..6d73782
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/pmc.h
> @@ -0,0 +1,9 @@
> +#ifndef __PMC_H__
> +#define __PMC_H__
> +extern void start_perf_counter(void);
> +extern void start_cycle_counter(void);
> +extern void stop_cycle_counter(void);
> +extern void stop_perf_counter(void);
> +extern unsigned int cycle_count(void);
> +extern unsigned int delay_sram;
> +#endif
> diff --git a/arch/arm/plat-omap/include/plat/sram.h
> b/arch/arm/plat-omap/include/plat/sram.h index 16a1b45..6019ed8 100644
> --- a/arch/arm/plat-omap/include/plat/sram.h
> +++ b/arch/arm/plat-omap/include/plat/sram.h
> @@ -69,6 +69,12 @@ extern u32 omap3_sram_configure_core_dpll(
>  			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
>  extern unsigned long omap3_sram_configure_core_dpll_sz;
>
> +extern unsigned int measure_sram_delay(unsigned int);
> +
> +extern u32 __sram_wait_delay(
> +		unsigned int);
> +extern unsigned long __sram_wait_delay_sz;
> +
>  #ifdef CONFIG_PM
>  extern void omap_push_sram_idle(void);
>  #else
> diff --git a/arch/arm/plat-omap/pmc.c b/arch/arm/plat-omap/pmc.c
> new file mode 100644
> index 0000000..11acf42
> --- /dev/null
> +++ b/arch/arm/plat-omap/pmc.c
> @@ -0,0 +1,47 @@
> +#include <linux/module.h>
> +
> +void start_perf_counter(void)
> +{
> +	unsigned int r1 = 0;
> +	asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
> +	r1 |= 0x1; /* enable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(start_perf_counter);
> +
> +void start_cycle_counter(void)
> +{
> +	unsigned int r2 = 0;
> +	r2 = 0x80000000; /* enable cycle counter only */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r2));
> +	return;
> +}
> +EXPORT_SYMBOL(start_cycle_counter);
> +
> +unsigned int cycle_count(void)
> +{
> +	unsigned int rd = 0;
> +	asm("mrc p15, 0, %0, c9, c13, 0" : "=r" (rd));
> +	return rd;
> +}
> +EXPORT_SYMBOL(cycle_count);
> +
> +
> +void stop_cycle_counter(void)
> +{
> +	unsigned int r3 = 0;
> +	r3 = 0x0; /* disable cycle counter */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r3));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_cycle_counter);
> +
> +void stop_perf_counter(void)
> +{
> +	unsigned int r1 = 0;
> +	r1 = 0x0; /* disable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_perf_counter);
Ok what happens if the PMU unit already is enabled by e.g. perfcounters or 
oprofile?
For example the perfcounter code tries to acquire the PMU lock before trying 
to use it. That might be good to use it here also.

Is pm.c the right place to put this code? This belongs more to a perf unit 
measurement code.

> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index 3e92366..4ed6a48 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -29,6 +29,10 @@
>  #include <plat/board.h>
>  #include <plat/cpu.h>
>
> +#ifdef CONFIG_ARCH_OMAP3
> +#include <plat/pmc.h>
> +#endif
> +
>  #include <plat/control.h>
>
>  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> @@ -423,6 +427,25 @@ static inline int omap34xx_sram_init(void)
>  }
>  #endif
>
> +
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned int measure_sram_delay(unsigned int loop)
> +{
> +	unsigned int start = 0, end = 0;
> +	void (*_omap3_sram_delay)(unsigned long);
> +	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> +						__sram_wait_delay_sz);
> +	start_perf_counter();
> +	start_cycle_counter();
> +	start = cycle_count();
> +	_omap3_sram_delay(loop);
> +	end = cycle_count();
> +	stop_cycle_counter();
> +	stop_perf_counter();
> +	return end - start;
> +}
> +#endif
> +
>  int __init omap_sram_init(void)
>  {
>  	omap_detect_sram();
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Nishanth Menon]

Dasgupta, Romit had written, on 12/11/2009 06:31 AM, the following:
>> Also Richard indicated that there might be a few tricky things with perf
>> Counters with specific devices - like EMU/HS/GP devices. It might need EMU
>> Domain for the values to pass through and there might be a yet-not-measured increase In power which could impact usage numbers and may need additional 
>> Code to switch off the domain correctly while hitting OFF/RET..
>>
> 
> Yes someone with EMU/HS could run and let us know. OTOH there won't be any
> increase in power as it is done only once during boot time after which the
> perfcounters are stopped.
> 
> By the way can you run this in 3630 and help us find what is the SRAM access
> delay? I am sure it should be lesser since it has a process improvement over 34xx.
> 
> 
will try on SDP3630 among the boards that I have around. meanwhile, 
would like an explanation to my previous comments also esp on the black 
magic "6" ;) - thanks.

-- 
Regards,
Nishanth Menon

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Romit Dasgupta]

Jean Pihet wrote:
> On Friday 11 December 2009 13:05:37 Reddy, Teerth wrote:
>> Reposting the patch with proper format
>>
>> From: Teerth Reddy <teerth@ti.com>
>>
>> This patch sets the dpll3 clock stabilization delay during
>> DVFS. The stabilization delay is calculated dynamically
>> using the ARM performance counter.
>> Currently 6us of SDRC stabilization value is used to get
>> the correct delay.
> That is a good thing to have! However the counters might already be in use, 
> cf. below
> 
Yes, we had a concern about that and not sure how soon the perf counter starts
up. We do this at very early boot code as a part of init_IRQ. much before
profile_init. So I am not sure if your concern is valid.

Thanks,
-Romit

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Kevin Hilman]

"Reddy, Teerth" <teerth@ti.com> writes:

> Reposting the patch with proper format
>
> From: Teerth Reddy <teerth@ti.com>
>
> This patch sets the dpll3 clock stabilization delay during
> DVFS. The stabilization delay is calculated dynamically 
> using the ARM performance counter.
> Currently 6us of SDRC stabilization value is used to get 
> the correct delay.

Needs more thorough changelog describing the problem being solved.

First, some general comments/questions:

PMC code is ARM generic and already largely exists in other places
(oprofile for one.)  So the use of the PMC will need to be generalized
as well as be shown not to interfere with other users (as raised
already by Jean Pihet.)

Couldn't you use a GPtimer for this?

> Signed-off-by: Romit Dasgupta <romit@ti.com>
> Signed-off-by: Teerth Reddy <teerth@ti.com>
> ---
>  mach-omap2/clock34xx.c        |   32 +++++++++++++---------------
>  mach-omap2/sram34xx.S         |    7 ++++++
>  plat-omap/Makefile            |    1
>  plat-omap/include/plat/pmc.h  |    9 ++++++++
>  plat-omap/include/plat/sram.h |    6 +++++
>  plat-omap/pmc.c               |   47 ++++++++++++++++++++++++++++++++++++++++++
>  plat-omap/sram.c              |   23 ++++++++++++++++++++
>  7 files changed, 108 insertions(+), 17 deletions(-)
>
>
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
> index da5bc1f..dd49938 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -37,6 +37,8 @@
>  #include <asm/div64.h>
>  #include <asm/clkdev.h>
>  
> +#include <plat/pmc.h>
> +
>  #include <plat/sdrc.h>
>  #include "clock.h"
>  #include "prm.h"
> @@ -46,6 +48,8 @@
>  
>  static const struct clkops clkops_noncore_dpll_ops;
>  
> +unsigned int delay_sram;
> +

static?  The only users I see are in this file.

>  static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
>  					    void __iomem **idlest_reg,
>  					    u8 *idlest_bit);
> @@ -333,14 +337,9 @@ static struct omap_clk omap34xx_clks[] = {
>  /* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
>  #define SDRC_MPURATE_SCALE		8
>  
> -/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined for */
> -#define SDRC_MPURATE_BASE_SHIFT		9
> -
> -/*
> - * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
> - * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
> - */
> -#define SDRC_MPURATE_LOOPS		96
> +/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
> +/* hardcoded currently */

hardcoded currently why?  "currently" makes it sound like there are
some other plans.  Would be useful to comment on what those are.

> +#define	SDRC_TIME_STABILIZE		6
>  
>  /*
>   * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
> @@ -870,16 +869,10 @@ static int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
>  		unlock_dll = 1;
>  	}
>  
> -	/*
> -	 * XXX This only needs to be done when the CPU frequency changes
> -	 */
> +	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
> +
>  	mpurate = arm_fck.rate / CYCLES_PER_MHZ;
> -	c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
> -	c += 1;  /* for safety */
> -	c *= SDRC_MPURATE_LOOPS;
> -	c >>= SDRC_MPURATE_SCALE;
> -	if (c == 0)
> -		c = 1;
> +	c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
>  
>  	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
>  		 validrate);
> @@ -1228,6 +1221,11 @@ int __init omap2_clk_init(void)
>  	vclk = clk_get(NULL, "virt_prcm_set");
>  	sclk = clk_get(NULL, "sys_ck");
>  #endif
> +
> +	/* Measure sram delay */
> +	delay_sram = measure_sram_delay(10000)/(10000*2);
> +	pr_debug("SRAM delay: %d\n", delay_sram);
> +
>  	return 0;
>  }
>  
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> index 82aa4a3..dc0ac72 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -298,3 +298,10 @@ core_m2_mask_val:
>  ENTRY(omap3_sram_configure_core_dpll_sz)
>  	.word	. - omap3_sram_configure_core_dpll
>  
> +ENTRY(__sram_wait_delay)
> +	subs 	r0, r0, #1
> +	bne	__sram_wait_delay
> +	bx	lr
> +
> +ENTRY(__sram_wait_delay_sz)
> +	.word	. - __sram_wait_delay
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 95f8413..aac43da 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>  # omap_device support (OMAP2+ only at the moment)
>  obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
>  obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
> +obj-$(CONFIG_ARCH_OMAP3) += pmc.o
>  
>  obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
>  obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
> diff --git a/arch/arm/plat-omap/include/plat/pmc.h b/arch/arm/plat-omap/include/plat/pmc.h
> new file mode 100644
> index 0000000..6d73782
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/pmc.h
> @@ -0,0 +1,9 @@
> +#ifndef __PMC_H__
> +#define __PMC_H__
> +extern void start_perf_counter(void);
> +extern void start_cycle_counter(void);
> +extern void stop_cycle_counter(void);
> +extern void stop_perf_counter(void);
> +extern unsigned int cycle_count(void);
> +extern unsigned int delay_sram;
> +#endif
> diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
> index 16a1b45..6019ed8 100644
> --- a/arch/arm/plat-omap/include/plat/sram.h
> +++ b/arch/arm/plat-omap/include/plat/sram.h
> @@ -69,6 +69,12 @@ extern u32 omap3_sram_configure_core_dpll(
>  			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
>  extern unsigned long omap3_sram_configure_core_dpll_sz;
>  
> +extern unsigned int measure_sram_delay(unsigned int);
> +
> +extern u32 __sram_wait_delay(
> +		unsigned int);
> +extern unsigned long __sram_wait_delay_sz;
> +
>  #ifdef CONFIG_PM
>  extern void omap_push_sram_idle(void);
>  #else
> diff --git a/arch/arm/plat-omap/pmc.c b/arch/arm/plat-omap/pmc.c
> new file mode 100644
> index 0000000..11acf42
> --- /dev/null
> +++ b/arch/arm/plat-omap/pmc.c
> @@ -0,0 +1,47 @@
> +#include <linux/module.h>
> +
> +void start_perf_counter(void)
> +{
> +	unsigned int r1 = 0;

blank line needed

> +	asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
> +	r1 |= 0x1; /* enable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(start_perf_counter);

export not needed (on any of these functions)

> +void start_cycle_counter(void)
> +{
> +	unsigned int r2 = 0;

blank line needed, assignment not necessary

> +	r2 = 0x80000000; /* enable cycle counter only */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r2));
> +	return;
> +}
> +EXPORT_SYMBOL(start_cycle_counter);
> +
> +unsigned int cycle_count(void)
> +{
> +	unsigned int rd = 0;

blank line

> +	asm("mrc p15, 0, %0, c9, c13, 0" : "=r" (rd));
> +	return rd;
> +}
> +EXPORT_SYMBOL(cycle_count);
> +
> +
> +void stop_cycle_counter(void)
> +{
> +	unsigned int r3 = 0;

blank line, assignment not needed

> +	r3 = 0x0; /* disable cycle counter */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r3));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_cycle_counter);
> +
> +void stop_perf_counter(void)
> +{
> +	unsigned int r1 = 0;

ditto

> +	r1 = 0x0; /* disable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_perf_counter);
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index 3e92366..4ed6a48 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -29,6 +29,10 @@
>  #include <plat/board.h>
>  #include <plat/cpu.h>
>  
> +#ifdef CONFIG_ARCH_OMAP3
> +#include <plat/pmc.h>
> +#endif
> +
>  #include <plat/control.h>
>  
>  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> @@ -423,6 +427,25 @@ static inline int omap34xx_sram_init(void)
>  }
>  #endif
>  
> +
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned int measure_sram_delay(unsigned int loop)
> +{
> +	unsigned int start = 0, end = 0;
> +	void (*_omap3_sram_delay)(unsigned long);
> +	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> +						__sram_wait_delay_sz);
> +	start_perf_counter();
> +	start_cycle_counter();
> +	start = cycle_count();
> +	_omap3_sram_delay(loop);
> +	end = cycle_count();
> +	stop_cycle_counter();
> +	stop_perf_counter();
> +	return end - start;
> +}
> +#endif
> +
>  int __init omap_sram_init(void)
>  {
>  	omap_detect_sram();
> --

Kevin

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Kevin Hilman]

Kevin Hilman <khilman@deeprootsystems.com> writes:

[...]

> PMC code is ARM generic and already largely exists in other places
> (oprofile for one.)  So the use of the PMC will need to be generalized
> as well as be shown not to interfere with other users (as raised
> already by Jean Pihet.)

Someone is already trying to generalize a PMC interface.  You might want
to follow this thread:

http://lists.infradead.org/pipermail/linux-arm-kernel/2009-December/005898.html

Kevin

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Nishanth Menon]

Menon, Nishanth had written, on 12/11/2009 07:42 AM, the following:
> Dasgupta, Romit had written, on 12/11/2009 06:31 AM, the following:
>>> Also Richard indicated that there might be a few tricky things with perf
>>> Counters with specific devices - like EMU/HS/GP devices. It might need EMU
>>> Domain for the values to pass through and there might be a yet-not-measured increase In power which could impact usage numbers and may need additional 
>>> Code to switch off the domain correctly while hitting OFF/RET..
>>>
>> Yes someone with EMU/HS could run and let us know. OTOH there won't be any
>> increase in power as it is done only once during boot time after which the
>> perfcounters are stopped.
>>
>> By the way can you run this in 3630 and help us find what is the SRAM access
>> delay? I am sure it should be lesser since it has a process improvement over 34xx.
>>
>>
> will try on SDP3630 among the boards that I have around. meanwhile, 
> would like an explanation to my previous comments also esp on the black 
> magic "6" ;) - thanks.
> 
Just realized -> clock framework changes are not in yet, anyways, with 
3630SDP (without my OPP series and on pm branch):
Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz
SRAM delay: 54 

Reprogramming SDRC clock to 400000000 Hz

-- 
Regards,
Nishanth Menon

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Romit Dasgupta]

Menon, Nishanth wrote:
> Menon, Nishanth had written, on 12/11/2009 07:42 AM, the following:
>> Dasgupta, Romit had written, on 12/11/2009 06:31 AM, the following:
>>>> Also Richard indicated that there might be a few tricky things with perf
>>>> Counters with specific devices - like EMU/HS/GP devices. It might need EMU
>>>> Domain for the values to pass through and there might be a yet-not-measured increase In power which could impact usage numbers and may need additional 
>>>> Code to switch off the domain correctly while hitting OFF/RET..
>>>>
>>> Yes someone with EMU/HS could run and let us know. OTOH there won't be any
>>> increase in power as it is done only once during boot time after which the
>>> perfcounters are stopped.
>>>
>>> By the way can you run this in 3630 and help us find what is the SRAM access
>>> delay? I am sure it should be lesser since it has a process improvement over 34xx.
>>>
>>>
>> will try on SDP3630 among the boards that I have around. meanwhile, 
>> would like an explanation to my previous comments also esp on the black 
>> magic "6" ;) - thanks.
>>
> Just realized -> clock framework changes are not in yet, anyways, with 
> 3630SDP (without my OPP series and on pm branch):
> Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz
> SRAM delay: 54 
> 
> Reprogramming SDRC clock to 400000000 Hz
> 
Thanks Nishanth. As expected the SRAM delay cycles are less compared to 3430.
However only 3 cycles!! ;-)

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Romit Dasgupta]

Kevin Hilman wrote:
> Kevin Hilman <khilman@deeprootsystems.com> writes:
> 
> [...]
> 
>> PMC code is ARM generic and already largely exists in other places
>> (oprofile for one.)  So the use of the PMC will need to be generalized
>> as well as be shown not to interfere with other users (as raised
>> already by Jean Pihet.)
> 
> Someone is already trying to generalize a PMC interface.  You might want
> to follow this thread:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2009-December/005898.html
> 
I tried to find the code in LO but I think it is not yet present. So with that
in mind may I propose that the pmc functions we introduce in plat-omap will be
present as __init code (so that we ensure no one uses it after the system
finishes booting up) as long as the pmc infrastructure is not available for
non-Oprofile uses? As I mentioned in an earlier thread, the pmc is used and
stopped very early during the kernel boot and AFAICT there should not be any
problem (probably we need to check its behavior on EMU/HS devices).

Thanks,
-Romit

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Kevin Hilman]

Romit Dasgupta <romit@ti.com> writes:

> Kevin Hilman wrote:
>> Kevin Hilman <khilman@deeprootsystems.com> writes:
>> 
>> [...]
>> 
>>> PMC code is ARM generic and already largely exists in other places
>>> (oprofile for one.)  So the use of the PMC will need to be generalized
>>> as well as be shown not to interfere with other users (as raised
>>> already by Jean Pihet.)
>> 
>> Someone is already trying to generalize a PMC interface.  You might want
>> to follow this thread:
>> 
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2009-December/005898.html
>> 
>
> I tried to find the code in LO but I think it is not yet present. 

Right, it is still being discussed.

> So with that in mind may I propose that the pmc functions we
> introduce in plat-omap will be present as __init code (so that we
> ensure no one uses it after the system finishes booting up) as long
> as the pmc infrastructure is not available for non-Oprofile uses? 

Personally, I don't like this idea.

IMHO, the PMC is not OMAP specific and does not belong in plat-omap
even as init code.  Either generalize and use existing PMC
infrastructure, or use a different timer.

You didn't answer my other question about whether or not a GPtimer
could be used for this.  You could quickly request/program/free a
GPtimer for this too using the omap_dm_timer* API.

> As I mentioned in an earlier thread, the pmc is used and stopped
> very early during the kernel boot and AFAICT there should not be any
> problem (probably we need to check its behavior on EMU/HS devices).

This isn't very convicing.  

Also, it's not just oprofile we have to be worried about.  The
perf/trace infrastructure arm is also using the PMC.  It will not be
uncommon to use perf on early boot/init and the use of the PMC in this
patch will clearly interfere with that.

In summary, the PMC is a shared resource and should be treated as such
using common code and common APIs.

Kevin

RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Dasgupta, Romit]

>>
>> I tried to find the code in LO but I think it is not yet present.

>Right, it is still being discussed.

>> So with that in mind may I propose that the pmc functions we
>> introduce in plat-omap will be present as __init code (so that we
>> ensure no one uses it after the system finishes booting up) as long
>> as the pmc infrastructure is not available for non-Oprofile uses?

>Personally, I don't like this idea.

>IMHO, the PMC is not OMAP specific and does not belong in plat-omap
>even as init code.  Either generalize and use existing PMC
>infrastructure, or use a different timer.

Ftrace/Oprofile/Perf are all initialized later during the boot process.
Actually if you see where we are using pmc it is in init_IRQ.
So until the pmc libraries I think it would be fair to use it. I do not see any problem.
Can you point out with any concrete code from the latest OMAP tree?

>You didn't answer my other question about whether or not a GPtimer
>could be used for this.  You could quickly request/program/free a
>GPtimer for this too using the omap_dm_timer* API.

May be possibe but IMHO it is ugly to setup and more complicated to use than the pmc approach. The other indirect way is like how bogomips is calculated. But that is a different story. We need accuracy in the delay and __may be__ interrupt latency would skew the results. If someone can implement it using Interrupt we would like to see how much access delay they get using irq technique.

>> As I mentioned in an earlier thread, the pmc is used and stopped
>> very early during the kernel boot and AFAICT there should not be any
>> problem (probably we need to check its behavior on EMU/HS devices).

>This isn't very convicing.

>Also, it's not just oprofile we have to be worried about.  The
>perf/trace infrastructure arm is also using the PMC.  It will not be
>uncommon to use perf on early boot/init and the use of the PMC in this
>patch will clearly interfere with that.
Wrong! See above.


>In summary, the PMC is a shared resource and should be treated as such
>using common code and common APIs. 

Where is it today?

Thanks,
-Romit

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Kevin Hilman]

"Dasgupta, Romit" <romit@ti.com> writes:

>>>
>>> I tried to find the code in LO but I think it is not yet present.
>
>>Right, it is still being discussed.
>
>>> So with that in mind may I propose that the pmc functions we
>>> introduce in plat-omap will be present as __init code (so that we
>>> ensure no one uses it after the system finishes booting up) as long
>>> as the pmc infrastructure is not available for non-Oprofile uses?
>
>>Personally, I don't like this idea.
>
>>IMHO, the PMC is not OMAP specific and does not belong in plat-omap
>>even as init code.  Either generalize and use existing PMC
>>infrastructure, or use a different timer.
>
> Ftrace/Oprofile/Perf are all initialized later during the boot
> process.  Actually if you see where we are using pmc it is in
> init_IRQ.  So until the pmc libraries I think it would be fair to
> use it. I do not see any problem.

I'm not against using the PMC, per se.  I'm against the way you've
coded it as an OMAP specific library.

> Can you point out with any concrete code from the latest OMAP tree?

The current PMC code in mainline is in oprofile and would need to be
generalized to be used by more users (e.g. oprofile, perf, your code,
etc.)

Jamie Iles has done the heavy lifting on this alrady for ARMv6[1], and
there are already discussions flowing on on linux-arm-kernel on how to
extend this for ARMv7.

A generalized PMC infrastructure is clearly needed (not just for your
code) so adding something that is OMAP specific is not the right
approach IMO.

>>You didn't answer my other question about whether or not a GPtimer
>>could be used for this.  You could quickly request/program/free a
>>GPtimer for this too using the omap_dm_timer* API.
>
> May be possibe but IMHO it is ugly to setup and more complicated to
> use than the pmc approach. 

What would be ugly?  All you need is a start, read and stop API.  This
is provided cleanly by the DM timer API.  

Here's an uncompiled, untested version of 'measure_sram_delay' using
the DM Timer API:

unsigned int measure_sram_delay(unsigned int loop)
{
	unsigned long start, end, flags;
	void (*_omap3_sram_delay)(unsigned long);
	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
						__sram_wait_delay_sz);

	gpt = omap_dm_timer_request();
	if (!gpt)
		pr_err("foo");
	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
	omap_dm_timer_set_load_start(gptimer, 0, 0);

        local_irq_save(flags);
        start = omap_dm_timer_read_counter(gpt);
	_omap3_sram_delay(loop);
        end = omap_dm_timer_read_counter(gpt);
        local_irq_restore(flags);
  
	omap_dm_timer_stop(gpt);
        omap_dm_timer_free(gpt);

	return end - start;
}

Since your needs are very short lived, I didn't bother setting
up any interrupt handling for timer overflow etc., I just start
the timer at zero.

> The other indirect way is like how
> bogomips is calculated. But that is a different story. We need
> accuracy in the delay and __may be__ interrupt latency would skew
> the results. If someone can implement it using Interrupt we would
> like to see how much access delay they get using irq technique.
>
>>> As I mentioned in an earlier thread, the pmc is used and stopped
>>> very early during the kernel boot and AFAICT there should not be any
>>> problem (probably we need to check its behavior on EMU/HS devices).
>
>>This isn't very convicing.
>
>>Also, it's not just oprofile we have to be worried about.  The
>>perf/trace infrastructure arm is also using the PMC.  It will not be
>>uncommon to use perf on early boot/init and the use of the PMC in this
>>patch will clearly interfere with that.
>
> Wrong! See above.

OK, I'm wrong about the init sequence.

However, in your approach, the SRAM delay timing *must* be done very early
in the init.  In a generalized approach, with a reserve mechanism it could
be done later, or not until needed.  

Depending on early init has implicit assumptions about other
subsystems whose ordering may change, and is in general not a good
idea.

>
>>In summary, the PMC is a shared resource and should be treated as such
>>using common code and common APIs. 
>
> Where is it today?

Under proposal and discussion[1].  If you want to use PMC, you'll need
to use a generalized approach.

IMO, an OMAP specific approach to PMC is unacceptable.

Kevin

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2009-December/006065.html

RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Reddy, Teerth]

Kevin,

> 
> Here's an uncompiled, untested version of 'measure_sram_delay' using
> the DM Timer API:
> 
> unsigned int measure_sram_delay(unsigned int loop)
> {
>     unsigned long start, end, flags;
>     void (*_omap3_sram_delay)(unsigned long);
>     _omap3_sram_delay = omap_sram_push(__sram_wait_delay,
>                                   __sram_wait_delay_sz);
> 
>     gpt = omap_dm_timer_request();
>     if (!gpt)
>           pr_err("foo");
>     omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
>     omap_dm_timer_set_load_start(gptimer, 0, 0);
> 
>         local_irq_save(flags);
>         start = omap_dm_timer_read_counter(gpt);
>     _omap3_sram_delay(loop);
>         end = omap_dm_timer_read_counter(gpt);
>         local_irq_restore(flags);
> 
>     omap_dm_timer_stop(gpt);
>         omap_dm_timer_free(gpt);
> 
>     return end - start;
> }
> 

I see one shortcoming with this approach. The DVFS happens even before the gp timers are initialized, while the kernel boots. I am trying to use gp timers and I came across this problem. I didn't see this issue with PMC. Please correct me if my understanding is wrong.

--Snip---
Kernel command line: mem=128M, console=ttyS0,115200n8 noinitrd root=/dev/nfs rw nfsroot=172.24.190.217:/data/nfs-share/teerth/target11,nolock,rsize=1024,wsize=1024 ip=dhcp devfs=mount
PID hash table entries: 512 (order: -1, 2048 bytes)
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
Memory: 128MB = 128MB total
Memory: 125780KB available (3496K code, 358K data, 128K init, 0K highmem)
SLUB: Genslabs=9, HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
Hierarchical RCU implementation.
NR_IRQS:402
Clocking rate (Crystal/Core/MPU): 26.0/332/500 MHz
Reprogramming SDRC clock to 332000000 Hz
------
 GPMC revision 5.0
IRQ: Found an INTC at 0xfa200000 (revision 4.0) with 96 interrupts
Total of 96 interrupts on 1 active controller
OMAP GPIO hardware version 2.5
OMAP clockevent source: GPTIMER1 at 32768 Hz
------
Console: colour dummy device 80x30
Calibrating delay loop... 498.87 BogoMIPS (lpj=1945600)
Mount-cache hash table entries: 512
CPU: Testing write buffer coherency: ok
regulator: core version 0.5

Regards
Teerth

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Kevin Hilman]

"Reddy, Teerth" <teerth@ti.com> writes:

> Kevin,
>
>> 
>> Here's an uncompiled, untested version of 'measure_sram_delay' using
>> the DM Timer API:
>> 
>> unsigned int measure_sram_delay(unsigned int loop)
>> {
>>     unsigned long start, end, flags;
>>     void (*_omap3_sram_delay)(unsigned long);
>>     _omap3_sram_delay = omap_sram_push(__sram_wait_delay,
>>                                   __sram_wait_delay_sz);
>> 
>>     gpt = omap_dm_timer_request();
>>     if (!gpt)
>>           pr_err("foo");
>>     omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
>>     omap_dm_timer_set_load_start(gptimer, 0, 0);
>> 
>>         local_irq_save(flags);
>>         start = omap_dm_timer_read_counter(gpt);
>>     _omap3_sram_delay(loop);
>>         end = omap_dm_timer_read_counter(gpt);
>>         local_irq_restore(flags);
>> 
>>     omap_dm_timer_stop(gpt);
>>         omap_dm_timer_free(gpt);
>> 
>>     return end - start;
>> }
>> 
>
> I see one shortcoming with this approach. The DVFS happens even
> before the gp timers are initialized, while the kernel boots.

This is a bug.  DVFS should be prevented until system is initialized.

Kevin

> I am trying to use gp timers and I came across this problem. I
> didn't see this issue with PMC. Please correct me if my
> understanding is wrong.

> --Snip---
> Kernel command line: mem=128M, console=ttyS0,115200n8 noinitrd root=/dev/nfs rw nfsroot=172.24.190.217:/data/nfs-share/teerth/target11,nolock,rsize=1024,wsize=1024 ip=dhcp devfs=mount
> PID hash table entries: 512 (order: -1, 2048 bytes)
> Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
> Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
> Memory: 128MB = 128MB total
> Memory: 125780KB available (3496K code, 358K data, 128K init, 0K highmem)
> SLUB: Genslabs=9, HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> Hierarchical RCU implementation.
> NR_IRQS:402
> Clocking rate (Crystal/Core/MPU): 26.0/332/500 MHz
> Reprogramming SDRC clock to 332000000 Hz
> ------
>  GPMC revision 5.0
> IRQ: Found an INTC at 0xfa200000 (revision 4.0) with 96 interrupts
> Total of 96 interrupts on 1 active controller
> OMAP GPIO hardware version 2.5
> OMAP clockevent source: GPTIMER1 at 32768 Hz
> ------
> Console: colour dummy device 80x30
> Calibrating delay loop... 498.87 BogoMIPS (lpj=1945600)
> Mount-cache hash table entries: 512
> CPU: Testing write buffer coherency: ok
> regulator: core version 0.5
>
> Regards
> Teerth

RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Sripathy, Vishwanath]

Kevin,

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Kevin Hilman
> Sent: Tuesday, December 22, 2009 9:26 PM
> To: Reddy, Teerth
> Cc: Dasgupta, Romit; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization
> delay
> 
> "Reddy, Teerth" <teerth@ti.com> writes:
> 
> > Kevin,
> >
> >>
> >> Here's an uncompiled, untested version of 'measure_sram_delay' using
> >> the DM Timer API:
> >>
> >> unsigned int measure_sram_delay(unsigned int loop)
> >> {
> >>     unsigned long start, end, flags;
> >>     void (*_omap3_sram_delay)(unsigned long);
> >>     _omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> >>                                   __sram_wait_delay_sz);
> >>
> >>     gpt = omap_dm_timer_request();
> >>     if (!gpt)
> >>           pr_err("foo");
> >>     omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> >>     omap_dm_timer_set_load_start(gptimer, 0, 0);
> >>
> >>         local_irq_save(flags);
> >>         start = omap_dm_timer_read_counter(gpt);
> >>     _omap3_sram_delay(loop);
> >>         end = omap_dm_timer_read_counter(gpt);
> >>         local_irq_restore(flags);
> >>
> >>     omap_dm_timer_stop(gpt);
> >>         omap_dm_timer_free(gpt);
> >>
> >>     return end - start;
> >> }
> >>
> >
> > I see one shortcoming with this approach. The DVFS happens even
> > before the gp timers are initialized, while the kernel boots.
> 
> This is a bug.  DVFS should be prevented until system is initialized.
> 


I think DVFS is triggered when CPU Freq driver gets initialized. Depending on the default governor cpufreq tries to scale the frequency which triggers DVFS. So do you mean to say that cpufreq initialization should be prevented till GPTimer is initialized?

> Kevin
> 
> > I am trying to use gp timers and I came across this problem. I
> > didn't see this issue with PMC. Please correct me if my
> > understanding is wrong.
> 
> > --Snip---
> > Kernel command line: mem=128M, console=ttyS0,115200n8 noinitrd
> root=/dev/nfs rw nfsroot=172.24.190.217:/data/nfs-
> share/teerth/target11,nolock,rsize=1024,wsize=1024 ip=dhcp devfs=mount
> > PID hash table entries: 512 (order: -1, 2048 bytes)
> > Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
> > Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
> > Memory: 128MB = 128MB total
> > Memory: 125780KB available (3496K code, 358K data, 128K init, 0K highmem)
> > SLUB: Genslabs=9, HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> > Hierarchical RCU implementation.
> > NR_IRQS:402
> > Clocking rate (Crystal/Core/MPU): 26.0/332/500 MHz
> > Reprogramming SDRC clock to 332000000 Hz
> > ------
> >  GPMC revision 5.0
> > IRQ: Found an INTC at 0xfa200000 (revision 4.0) with 96 interrupts
> > Total of 96 interrupts on 1 active controller
> > OMAP GPIO hardware version 2.5
> > OMAP clockevent source: GPTIMER1 at 32768 Hz
> > ------
> > Console: colour dummy device 80x30
> > Calibrating delay loop... 498.87 BogoMIPS (lpj=1945600)
> > Mount-cache hash table entries: 512
> > CPU: Testing write buffer coherency: ok
> > regulator: core version 0.5
> >
> > Regards
> > Teerth
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Kevin Hilman]

"Sripathy, Vishwanath" <vishwanath.bs@ti.com> writes:

> Kevin,
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Kevin Hilman
>> Sent: Tuesday, December 22, 2009 9:26 PM
>> To: Reddy, Teerth
>> Cc: Dasgupta, Romit; linux-omap@vger.kernel.org
>> Subject: Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization
>> delay
>> 
>> "Reddy, Teerth" <teerth@ti.com> writes:
>> 
>> > Kevin,
>> >
>> >>
>> >> Here's an uncompiled, untested version of 'measure_sram_delay' using
>> >> the DM Timer API:
>> >>
>> >> unsigned int measure_sram_delay(unsigned int loop)
>> >> {
>> >>     unsigned long start, end, flags;
>> >>     void (*_omap3_sram_delay)(unsigned long);
>> >>     _omap3_sram_delay = omap_sram_push(__sram_wait_delay,
>> >>                                   __sram_wait_delay_sz);
>> >>
>> >>     gpt = omap_dm_timer_request();
>> >>     if (!gpt)
>> >>           pr_err("foo");
>> >>     omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
>> >>     omap_dm_timer_set_load_start(gptimer, 0, 0);
>> >>
>> >>         local_irq_save(flags);
>> >>         start = omap_dm_timer_read_counter(gpt);
>> >>     _omap3_sram_delay(loop);
>> >>         end = omap_dm_timer_read_counter(gpt);
>> >>         local_irq_restore(flags);
>> >>
>> >>     omap_dm_timer_stop(gpt);
>> >>         omap_dm_timer_free(gpt);
>> >>
>> >>     return end - start;
>> >> }
>> >>
>> >
>> > I see one shortcoming with this approach. The DVFS happens even
>> > before the gp timers are initialized, while the kernel boots.
>> 
>> This is a bug.  DVFS should be prevented until system is initialized.
>> 
>
>
> I think DVFS is triggered when CPU Freq driver gets
> initialized. Depending on the default governor cpufreq tries to
> scale the frequency which triggers DVFS.

Correct.

> So do you mean to say that cpufreq initialization should be
> prevented till GPTimer is initialized?

Basically, yes.  

You need to calibrate the delay before you can do DVFS, so what I mean
is that you need to request your GPtimer, and do your delay
measurements using the GPtimer before you enable DVFS (register
CPUfreq.)

Also, please be sure to stop/free the GPtimer when done, otherwise
it will prevent idle.

Kevin

[PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Reddy, Teerth]

Dynamic calculation of SDRC clock stabilization delay using gptimer as recommended by Kevin.


>From e5e82efbf6f736984668cc5e94d62635ed4de05e Mon Sep 17 00:00:00 2001
From: Teerth Reddy <teerth@ti.com>
Date: Wed, 23 Dec 2009 18:40:34 +0530
Subject: [PATCH] Dynamic Calculation of SDRC clock stabilization delay

The patch has the changes to calculate the dpll3 clock
stabilization delay dynamically. The SRAM delay is
calibrated during bootup using the gptimers and used while
calculating the stabilization delay. By using the dynamic
method the dependency on the type of cache being used is
removed. Hence there is no need of loop based calculation.

TO DO: However the value of 6us for SDRC to stabilize has
been hardcoded currently. The formula used to calculate
this value gives a very small number thus causing
instability. The right formula will be used which will
work across all boards.

Signed-off-by: Teerth Reddy <teerth@ti.com>
Signed-off-by: Romit Dasgupta <romit@ti.com>
---
 arch/arm/mach-omap2/clock34xx.c        |   21 +++++++++------
 arch/arm/mach-omap2/clock34xx.h        |    2 +
 arch/arm/mach-omap2/clock34xx_data.c   |    4 +++
 arch/arm/mach-omap2/sram34xx.S         |   25 ++++++++++++++++++
 arch/arm/plat-omap/include/plat/sram.h |    5 +++
 arch/arm/plat-omap/sram.c              |   43 ++++++++++++++++++++++++++++++++
 6 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index d9329e2..6d87419 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -56,9 +56,18 @@
  */
 #define DPLL5_FREQ_FOR_USBHOST		120000000
 
+/*
+ * SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us
+ * TO DO: Use formula to calculate across all platforms
+ */
+#define		SDRC_TIME_STABILIZE	6
+
 /* needed by omap3_core_dpll_m2_set_rate() */
 struct clk *sdrc_ick_p, *arm_fck_p;
 
+/* SRAM delay required for sdrc clk stab delay calculation */
+unsigned int delay_sram;
+
 /**
  * omap3430es2_clk_ssi_find_idlest - return CM_IDLEST info for SSI
  * @clk: struct clk * being enabled
@@ -215,16 +224,10 @@ int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
 		unlock_dll = 1;
 	}
 
-	/*
-	 * XXX This only needs to be done when the CPU frequency changes
-	 */
+	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
+
 	_mpurate = arm_fck_p->rate / CYCLES_PER_MHZ;
-	c = (_mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
-	c += 1;  /* for safety */
-	c *= SDRC_MPURATE_LOOPS;
-	c >>= SDRC_MPURATE_SCALE;
-	if (c == 0)
-		c = 1;
+	c = ((SDRC_TIME_STABILIZE * _mpurate) / (delay_sram * 2));
 
 	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
 		 validrate);
diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
index 9a2c07e..1bc5044 100644
--- a/arch/arm/mach-omap2/clock34xx.h
+++ b/arch/arm/mach-omap2/clock34xx.h
@@ -21,4 +21,6 @@ extern const struct clkops clkops_omap3430es2_hsotgusb_wait;
 extern const struct clkops clkops_omap3430es2_dss_usbhost_wait;
 extern const struct clkops clkops_noncore_dpll_ops;
 
+extern unsigned int delay_sram;
+
 #endif
diff --git a/arch/arm/mach-omap2/clock34xx_data.c b/arch/arm/mach-omap2/clock34xx_data.c
index 8bdcc9c..e5d0941 100644
--- a/arch/arm/mach-omap2/clock34xx_data.c
+++ b/arch/arm/mach-omap2/clock34xx_data.c
@@ -22,6 +22,7 @@
 
 #include <plat/control.h>
 #include <plat/clkdev_omap.h>
+#include <plat/sram.h>
 
 #include "clock.h"
 #include "clock34xx.h"
@@ -3285,5 +3286,8 @@ int __init omap2_clk_init(void)
 	sdrc_ick_p = clk_get(NULL, "sdrc_ick");
 	arm_fck_p = clk_get(NULL, "arm_fck");
 
+	/* Measure sram delay */
+	delay_sram = measure_sram_delay(10000);
+	pr_debug("SRAM delay: %d\n", delay_sram);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index de99ba2..b4fba50 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -68,6 +68,10 @@
 /* CM_CLKSEL1_PLL bit settings */
 #define CORE_DPLL_CLKOUT_DIV_SHIFT	0x1b
 
+#define	GPTIMER10_TCRR_PHY			0x48086028
+
+#define	GPTIMER10_TCRR	OMAP2_L4_IO_ADDRESS(GPTIMER10_TCRR_PHY)
+
 /*
  * omap3_sram_configure_core_dpll - change DPLL3 M2 divider
  *
@@ -313,3 +317,24 @@ core_m2_mask_val:
 ENTRY(omap3_sram_configure_core_dpll_sz)
 	.word	. - omap3_sram_configure_core_dpll
 
+ENTRY(__sram_wait_delay)
+	stmfd	sp!, {r1-r12, lr}	@ store regs to stack
+ 	ldr	r2, gptimer10_counter
+	ldr	r3, [r2]
+
+loop1:
+	subs 	r0, r0, #1
+	bne	loop1
+
+	isb
+	ldr	r4, [r2]
+	subs	r5, r4, r3
+
+	mov 	r0, r5 			@ return value
+	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
+
+gptimer10_counter:
+	.word	GPTIMER10_TCRR
+
+ENTRY(__sram_wait_delay_sz)
+	.word	. - __sram_wait_delay
diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
index 16a1b45..7d0be2a 100644
--- a/arch/arm/plat-omap/include/plat/sram.h
+++ b/arch/arm/plat-omap/include/plat/sram.h
@@ -69,6 +69,11 @@ extern u32 omap3_sram_configure_core_dpll(
 			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
 extern unsigned long omap3_sram_configure_core_dpll_sz;
 
+extern unsigned int measure_sram_delay(unsigned int);
+
+extern u32 __sram_wait_delay(unsigned int);
+extern unsigned long __sram_wait_delay_sz;
+
 #ifdef CONFIG_PM
 extern void omap_push_sram_idle(void);
 #else
diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
index d8d5094..32bedda 100644
--- a/arch/arm/plat-omap/sram.c
+++ b/arch/arm/plat-omap/sram.c
@@ -30,6 +30,9 @@
 #include <plat/cpu.h>
 #include <plat/vram.h>
 
+#include <linux/clk.h>
+#include <plat/dmtimer.h>
+
 #include <plat/control.h>
 
 #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
@@ -437,11 +440,51 @@ static inline int omap34xx_sram_init(void)
 }
 #endif
 
+
+#ifdef CONFIG_ARCH_OMAP3
+unsigned long (*_omap3_sram_delay)(unsigned long);
+unsigned int  measure_sram_delay(unsigned int loop)
+{
+	static struct omap_dm_timer *gpt;
+	unsigned long flags, diff = 0, gt_rate, mpurate;
+	unsigned int delay_sram, error_gain;
+
+	omap_dm_timer_init();
+	gpt = omap_dm_timer_request_specific(10);
+	if (!gpt)
+		pr_err("Could not get the gptimer\n");
+	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
+
+	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
+	omap_dm_timer_set_load_start(gpt, 0, 0);
+
+	local_irq_save(flags);
+	diff = _omap3_sram_delay(loop);
+	local_irq_restore(flags);
+
+	omap_dm_timer_stop(gpt);
+	omap_dm_timer_free(gpt);
+
+	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
+
+	/* calculate the sram delay */
+	delay_sram = ((((mpurate/1000000) / (gt_rate/1000000)) * diff) / 20000);
+
+	error_gain = ((mpurate/1000000) / (gt_rate/1000000));
+	delay_sram = delay_sram + error_gain;
+
+	return delay_sram;
+}
+#endif
+
 int __init omap_sram_init(void)
 {
 	omap_detect_sram();
 	omap_map_sram();
 
+	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
+						__sram_wait_delay_sz);
+
 	if (!(cpu_class_is_omap2()))
 		omap1_sram_init();
 	else if (cpu_is_omap242x())

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Romit Dasgupta]

> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned long (*_omap3_sram_delay)(unsigned long);
> +unsigned int  measure_sram_delay(unsigned int loop)
> +{
> +	static struct omap_dm_timer *gpt;
> +	unsigned long flags, diff = 0, gt_rate, mpurate;
> +	unsigned int delay_sram, error_gain;
> +
> +	omap_dm_timer_init();
> +	gpt = omap_dm_timer_request_specific(10);
> +	if (!gpt)
> +		pr_err("Could not get the gptimer\n");
> +	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> +
> +	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> +	omap_dm_timer_set_load_start(gpt, 0, 0);
> +
> +	local_irq_save(flags);
> +	diff = _omap3_sram_delay(loop);
> +	local_irq_restore(flags);
> +
> +	omap_dm_timer_stop(gpt);
> +	omap_dm_timer_free(gpt);
> +
> +	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
> +
> +	/* calculate the sram delay */
> +	delay_sram = ((((mpurate/1000000) / (gt_rate/1000000)) * diff) / 20000);
Can remove the 1000000 from the denominators as they cancel out.
> +
> +	error_gain = ((mpurate/1000000) / (gt_rate/1000000));
Same as before.
> +	delay_sram = delay_sram + error_gain;
> +
> +	return delay_sram;
> +}

RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Reddy, Teerth]

Thanks Romit..

Will fix and send the patch.

Regards
Teerth

> -----Original Message-----
> From: Dasgupta, Romit
> Sent: Wednesday, December 23, 2009 8:02 PM
> To: Reddy, Teerth
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock
> stabilization delay
> 
> > +#ifdef CONFIG_ARCH_OMAP3
> > +unsigned long (*_omap3_sram_delay)(unsigned long);
> > +unsigned int  measure_sram_delay(unsigned int loop)
> > +{
> > +	static struct omap_dm_timer *gpt;
> > +	unsigned long flags, diff = 0, gt_rate, mpurate;
> > +	unsigned int delay_sram, error_gain;
> > +
> > +	omap_dm_timer_init();
> > +	gpt = omap_dm_timer_request_specific(10);
> > +	if (!gpt)
> > +		pr_err("Could not get the gptimer\n");
> > +	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> > +
> > +	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> > +	omap_dm_timer_set_load_start(gpt, 0, 0);
> > +
> > +	local_irq_save(flags);
> > +	diff = _omap3_sram_delay(loop);
> > +	local_irq_restore(flags);
> > +
> > +	omap_dm_timer_stop(gpt);
> > +	omap_dm_timer_free(gpt);
> > +
> > +	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
> > +
> > +	/* calculate the sram delay */
> > +	delay_sram = ((((mpurate/1000000) / (gt_rate/1000000)) * diff) /
> 20000);
> Can remove the 1000000 from the denominators as they cancel out.
> > +
> > +	error_gain = ((mpurate/1000000) / (gt_rate/1000000));
> Same as before.
> > +	delay_sram = delay_sram + error_gain;
> > +
> > +	return delay_sram;
> > +}

[PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Reddy, Teerth]

>From e5e82efbf6f736984668cc5e94d62635ed4de05e Mon Sep 17 00:00:00 2001
From: Teerth Reddy <teerth@ti.com>
Date: Wed, 23 Dec 2009 18:40:34 +0530
Subject: [PATCH] Dynamic Calculation of SDRC clock stabilization delay

The patch has the changes to calculate the dpll3 clock
stabilization delay dynamically. The SRAM delay is
calibrated during bootup using the gptimers and used while
calculating the stabilization delay. By using the dynamic
method the dependency on the type of cache being used is
removed. Hence there is no need of loop based calculation.

TO DO: However the value of 6us for SDRC to stabilize has
been hardcoded currently. The formula used to calculate
this value gives a very small number thus causing
instability. The right formula will be used which will
work across all boards.

Signed-off-by: Teerth Reddy <teerth@ti.com>
Signed-off-by: Romit Dasgupta <romit@ti.com>
---
 arch/arm/mach-omap2/clock34xx.c        |   21 +++++++++------
 arch/arm/mach-omap2/clock34xx.h        |    2 +
 arch/arm/mach-omap2/clock34xx_data.c   |    4 +++
 arch/arm/mach-omap2/sram34xx.S         |   25 ++++++++++++++++++
 arch/arm/plat-omap/include/plat/sram.h |    5 +++
 arch/arm/plat-omap/sram.c              |   43 ++++++++++++++++++++++++++++++++
 6 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index d9329e2..6d87419 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -56,9 +56,18 @@
  */
 #define DPLL5_FREQ_FOR_USBHOST		120000000
 
+/*
+ * SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us
+ * TO DO: Use formula to calculate across all platforms
+ */
+#define		SDRC_TIME_STABILIZE	6
+
 /* needed by omap3_core_dpll_m2_set_rate() */
 struct clk *sdrc_ick_p, *arm_fck_p;
 
+/* SRAM delay required for sdrc clk stab delay calculation */
+unsigned int delay_sram;
+
 /**
  * omap3430es2_clk_ssi_find_idlest - return CM_IDLEST info for SSI
  * @clk: struct clk * being enabled
@@ -215,16 +224,10 @@ int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
 		unlock_dll = 1;
 	}
 
-	/*
-	 * XXX This only needs to be done when the CPU frequency changes
-	 */
+	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
+
 	_mpurate = arm_fck_p->rate / CYCLES_PER_MHZ;
-	c = (_mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
-	c += 1;  /* for safety */
-	c *= SDRC_MPURATE_LOOPS;
-	c >>= SDRC_MPURATE_SCALE;
-	if (c == 0)
-		c = 1;
+	c = ((SDRC_TIME_STABILIZE * _mpurate) / (delay_sram * 2));
 
 	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
 		 validrate);
diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
index 9a2c07e..1bc5044 100644
--- a/arch/arm/mach-omap2/clock34xx.h
+++ b/arch/arm/mach-omap2/clock34xx.h
@@ -21,4 +21,6 @@ extern const struct clkops clkops_omap3430es2_hsotgusb_wait;
 extern const struct clkops clkops_omap3430es2_dss_usbhost_wait;
 extern const struct clkops clkops_noncore_dpll_ops;
 
+extern unsigned int delay_sram;
+
 #endif
diff --git a/arch/arm/mach-omap2/clock34xx_data.c b/arch/arm/mach-omap2/clock34xx_data.c
index 8bdcc9c..e5d0941 100644
--- a/arch/arm/mach-omap2/clock34xx_data.c
+++ b/arch/arm/mach-omap2/clock34xx_data.c
@@ -22,6 +22,7 @@
 
 #include <plat/control.h>
 #include <plat/clkdev_omap.h>
+#include <plat/sram.h>
 
 #include "clock.h"
 #include "clock34xx.h"
@@ -3285,5 +3286,8 @@ int __init omap2_clk_init(void)
 	sdrc_ick_p = clk_get(NULL, "sdrc_ick");
 	arm_fck_p = clk_get(NULL, "arm_fck");
 
+	/* Measure sram delay */
+	delay_sram = measure_sram_delay(10000);
+	pr_debug("SRAM delay: %d\n", delay_sram);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index de99ba2..b4fba50 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -68,6 +68,10 @@
 /* CM_CLKSEL1_PLL bit settings */
 #define CORE_DPLL_CLKOUT_DIV_SHIFT	0x1b
 
+#define	GPTIMER10_TCRR_PHY			0x48086028
+
+#define	GPTIMER10_TCRR	OMAP2_L4_IO_ADDRESS(GPTIMER10_TCRR_PHY)
+
 /*
  * omap3_sram_configure_core_dpll - change DPLL3 M2 divider
  *
@@ -313,3 +317,24 @@ core_m2_mask_val:
 ENTRY(omap3_sram_configure_core_dpll_sz)
 	.word	. - omap3_sram_configure_core_dpll
 
+ENTRY(__sram_wait_delay)
+	stmfd	sp!, {r1-r12, lr}	@ store regs to stack
+ 	ldr	r2, gptimer10_counter
+	ldr	r3, [r2]
+
+loop1:
+	subs 	r0, r0, #1
+	bne	loop1
+
+	isb
+	ldr	r4, [r2]
+	subs	r5, r4, r3
+
+	mov 	r0, r5 			@ return value
+	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
+
+gptimer10_counter:
+	.word	GPTIMER10_TCRR
+
+ENTRY(__sram_wait_delay_sz)
+	.word	. - __sram_wait_delay
diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
index 16a1b45..7d0be2a 100644
--- a/arch/arm/plat-omap/include/plat/sram.h
+++ b/arch/arm/plat-omap/include/plat/sram.h
@@ -69,6 +69,11 @@ extern u32 omap3_sram_configure_core_dpll(
 			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
 extern unsigned long omap3_sram_configure_core_dpll_sz;
 
+extern unsigned int measure_sram_delay(unsigned int);
+
+extern u32 __sram_wait_delay(unsigned int);
+extern unsigned long __sram_wait_delay_sz;
+
 #ifdef CONFIG_PM
 extern void omap_push_sram_idle(void);
 #else
diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
index d8d5094..ddbdaaf 100644
--- a/arch/arm/plat-omap/sram.c
+++ b/arch/arm/plat-omap/sram.c
@@ -30,6 +30,9 @@
 #include <plat/cpu.h>
 #include <plat/vram.h>
 
+#include <linux/clk.h>
+#include <plat/dmtimer.h>
+
 #include <plat/control.h>
 
 #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
@@ -437,11 +440,51 @@ static inline int omap34xx_sram_init(void)
 }
 #endif
 
+
+#ifdef CONFIG_ARCH_OMAP3
+unsigned long (*_omap3_sram_delay)(unsigned long);
+unsigned int  measure_sram_delay(unsigned int loop)
+{
+	static struct omap_dm_timer *gpt;
+	unsigned long flags, diff = 0, gt_rate, mpurate;
+	unsigned int delay_sram, error_gain;
+
+	omap_dm_timer_init();
+	gpt = omap_dm_timer_request_specific(10);
+	if (!gpt)
+		pr_err("Could not get the gptimer\n");
+	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
+
+	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
+	omap_dm_timer_set_load_start(gpt, 0, 0);
+
+	local_irq_save(flags);
+	diff = _omap3_sram_delay(loop);
+	local_irq_restore(flags);
+
+	omap_dm_timer_stop(gpt);
+	omap_dm_timer_free(gpt);
+
+	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
+
+	/* calculate the sram delay */
+	delay_sram = (((mpurate / gt_rate) * diff) / 20000);
+
+	error_gain = mpurate / gt_rate;
+	delay_sram = delay_sram + error_gain;
+
+	return delay_sram;
+}
+#endif
+
 int __init omap_sram_init(void)
 {
 	omap_detect_sram();
 	omap_map_sram();
 
+	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
+						__sram_wait_delay_sz);
+
 	if (!(cpu_class_is_omap2()))
 		omap1_sram_init();
 	else if (cpu_is_omap242x())

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Romit Dasgupta]

Teerth, one more correction/improvement below:
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index d8d5094..ddbdaaf 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned long (*_omap3_sram_delay)(unsigned long);
> +unsigned int  measure_sram_delay(unsigned int loop)
> +{
> +	static struct omap_dm_timer *gpt;
> +	unsigned long flags, diff = 0, gt_rate, mpurate;
> +	unsigned int delay_sram, error_gain;
> +
> +	omap_dm_timer_init();
> +	gpt = omap_dm_timer_request_specific(10);
> +	if (!gpt)
> +		pr_err("Could not get the gptimer\n");
> +	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> +
> +	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> +	omap_dm_timer_set_load_start(gpt, 0, 0);
> +
> +	local_irq_save(flags);
> +	diff = _omap3_sram_delay(loop);
> +	local_irq_restore(flags);
> +
> +	omap_dm_timer_stop(gpt);
> +	omap_dm_timer_free(gpt);
> +
> +	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
> +
> +	/* calculate the sram delay */
> +	delay_sram = (((mpurate / gt_rate) * diff) / 20000);
Instead of 20000 it should be (loop * 2).
> +
> +	error_gain = mpurate / gt_rate;
> +	delay_sram = delay_sram + error_gain;
> +
> +	return delay_sram;
> +}
> +#endif
> 

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Tony Lindgren]

* Reddy, Teerth <teerth@ti.com> [091223 21:31]:
> From e5e82efbf6f736984668cc5e94d62635ed4de05e Mon Sep 17 00:00:00 2001
> From: Teerth Reddy <teerth@ti.com>
> Date: Wed, 23 Dec 2009 18:40:34 +0530
> Subject: [PATCH] Dynamic Calculation of SDRC clock stabilization delay
> 
> The patch has the changes to calculate the dpll3 clock
> stabilization delay dynamically. The SRAM delay is
> calibrated during bootup using the gptimers and used while
> calculating the stabilization delay. By using the dynamic
> method the dependency on the type of cache being used is
> removed. Hence there is no need of loop based calculation.
> 
> TO DO: However the value of 6us for SDRC to stabilize has
> been hardcoded currently. The formula used to calculate
> this value gives a very small number thus causing
> instability. The right formula will be used which will
> work across all boards.

Why does this patch have "PM" in the subject?

This patch should not have anything to do with the pm branch,
it's obviously clock + SRAM related. All that code is in the
mainline kernel.

Please stop tagging patches as "PM" unless they really are
pm related.

Regards,

Tony

 
> Signed-off-by: Teerth Reddy <teerth@ti.com>
> Signed-off-by: Romit Dasgupta <romit@ti.com>
> ---
>  arch/arm/mach-omap2/clock34xx.c        |   21 +++++++++------
>  arch/arm/mach-omap2/clock34xx.h        |    2 +
>  arch/arm/mach-omap2/clock34xx_data.c   |    4 +++
>  arch/arm/mach-omap2/sram34xx.S         |   25 ++++++++++++++++++
>  arch/arm/plat-omap/include/plat/sram.h |    5 +++
>  arch/arm/plat-omap/sram.c              |   43 ++++++++++++++++++++++++++++++++
>  6 files changed, 91 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
> index d9329e2..6d87419 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -56,9 +56,18 @@
>   */
>  #define DPLL5_FREQ_FOR_USBHOST		120000000
>  
> +/*
> + * SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us
> + * TO DO: Use formula to calculate across all platforms
> + */
> +#define		SDRC_TIME_STABILIZE	6
> +
>  /* needed by omap3_core_dpll_m2_set_rate() */
>  struct clk *sdrc_ick_p, *arm_fck_p;
>  
> +/* SRAM delay required for sdrc clk stab delay calculation */
> +unsigned int delay_sram;
> +
>  /**
>   * omap3430es2_clk_ssi_find_idlest - return CM_IDLEST info for SSI
>   * @clk: struct clk * being enabled
> @@ -215,16 +224,10 @@ int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
>  		unlock_dll = 1;
>  	}
>  
> -	/*
> -	 * XXX This only needs to be done when the CPU frequency changes
> -	 */
> +	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
> +
>  	_mpurate = arm_fck_p->rate / CYCLES_PER_MHZ;
> -	c = (_mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
> -	c += 1;  /* for safety */
> -	c *= SDRC_MPURATE_LOOPS;
> -	c >>= SDRC_MPURATE_SCALE;
> -	if (c == 0)
> -		c = 1;
> +	c = ((SDRC_TIME_STABILIZE * _mpurate) / (delay_sram * 2));
>  
>  	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
>  		 validrate);
> diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
> index 9a2c07e..1bc5044 100644
> --- a/arch/arm/mach-omap2/clock34xx.h
> +++ b/arch/arm/mach-omap2/clock34xx.h
> @@ -21,4 +21,6 @@ extern const struct clkops clkops_omap3430es2_hsotgusb_wait;
>  extern const struct clkops clkops_omap3430es2_dss_usbhost_wait;
>  extern const struct clkops clkops_noncore_dpll_ops;
>  
> +extern unsigned int delay_sram;
> +
>  #endif
> diff --git a/arch/arm/mach-omap2/clock34xx_data.c b/arch/arm/mach-omap2/clock34xx_data.c
> index 8bdcc9c..e5d0941 100644
> --- a/arch/arm/mach-omap2/clock34xx_data.c
> +++ b/arch/arm/mach-omap2/clock34xx_data.c
> @@ -22,6 +22,7 @@
>  
>  #include <plat/control.h>
>  #include <plat/clkdev_omap.h>
> +#include <plat/sram.h>
>  
>  #include "clock.h"
>  #include "clock34xx.h"
> @@ -3285,5 +3286,8 @@ int __init omap2_clk_init(void)
>  	sdrc_ick_p = clk_get(NULL, "sdrc_ick");
>  	arm_fck_p = clk_get(NULL, "arm_fck");
>  
> +	/* Measure sram delay */
> +	delay_sram = measure_sram_delay(10000);
> +	pr_debug("SRAM delay: %d\n", delay_sram);
>  	return 0;
>  }
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> index de99ba2..b4fba50 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -68,6 +68,10 @@
>  /* CM_CLKSEL1_PLL bit settings */
>  #define CORE_DPLL_CLKOUT_DIV_SHIFT	0x1b
>  
> +#define	GPTIMER10_TCRR_PHY			0x48086028
> +
> +#define	GPTIMER10_TCRR	OMAP2_L4_IO_ADDRESS(GPTIMER10_TCRR_PHY)
> +
>  /*
>   * omap3_sram_configure_core_dpll - change DPLL3 M2 divider
>   *
> @@ -313,3 +317,24 @@ core_m2_mask_val:
>  ENTRY(omap3_sram_configure_core_dpll_sz)
>  	.word	. - omap3_sram_configure_core_dpll
>  
> +ENTRY(__sram_wait_delay)
> +	stmfd	sp!, {r1-r12, lr}	@ store regs to stack
> + 	ldr	r2, gptimer10_counter
> +	ldr	r3, [r2]
> +
> +loop1:
> +	subs 	r0, r0, #1
> +	bne	loop1
> +
> +	isb
> +	ldr	r4, [r2]
> +	subs	r5, r4, r3
> +
> +	mov 	r0, r5 			@ return value
> +	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
> +
> +gptimer10_counter:
> +	.word	GPTIMER10_TCRR
> +
> +ENTRY(__sram_wait_delay_sz)
> +	.word	. - __sram_wait_delay
> diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
> index 16a1b45..7d0be2a 100644
> --- a/arch/arm/plat-omap/include/plat/sram.h
> +++ b/arch/arm/plat-omap/include/plat/sram.h
> @@ -69,6 +69,11 @@ extern u32 omap3_sram_configure_core_dpll(
>  			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
>  extern unsigned long omap3_sram_configure_core_dpll_sz;
>  
> +extern unsigned int measure_sram_delay(unsigned int);
> +
> +extern u32 __sram_wait_delay(unsigned int);
> +extern unsigned long __sram_wait_delay_sz;
> +
>  #ifdef CONFIG_PM
>  extern void omap_push_sram_idle(void);
>  #else
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index d8d5094..ddbdaaf 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -30,6 +30,9 @@
>  #include <plat/cpu.h>
>  #include <plat/vram.h>
>  
> +#include <linux/clk.h>
> +#include <plat/dmtimer.h>
> +
>  #include <plat/control.h>
>  
>  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> @@ -437,11 +440,51 @@ static inline int omap34xx_sram_init(void)
>  }
>  #endif
>  
> +
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned long (*_omap3_sram_delay)(unsigned long);
> +unsigned int  measure_sram_delay(unsigned int loop)
> +{
> +	static struct omap_dm_timer *gpt;
> +	unsigned long flags, diff = 0, gt_rate, mpurate;
> +	unsigned int delay_sram, error_gain;
> +
> +	omap_dm_timer_init();
> +	gpt = omap_dm_timer_request_specific(10);
> +	if (!gpt)
> +		pr_err("Could not get the gptimer\n");
> +	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> +
> +	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> +	omap_dm_timer_set_load_start(gpt, 0, 0);
> +
> +	local_irq_save(flags);
> +	diff = _omap3_sram_delay(loop);
> +	local_irq_restore(flags);
> +
> +	omap_dm_timer_stop(gpt);
> +	omap_dm_timer_free(gpt);
> +
> +	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
> +
> +	/* calculate the sram delay */
> +	delay_sram = (((mpurate / gt_rate) * diff) / 20000);
> +
> +	error_gain = mpurate / gt_rate;
> +	delay_sram = delay_sram + error_gain;
> +
> +	return delay_sram;
> +}
> +#endif
> +
>  int __init omap_sram_init(void)
>  {
>  	omap_detect_sram();
>  	omap_map_sram();
>  
> +	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> +						__sram_wait_delay_sz);
> +
>  	if (!(cpu_class_is_omap2()))
>  		omap1_sram_init();
>  	else if (cpu_is_omap242x())
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Kevin Hilman]

"Reddy, Teerth" <teerth@ti.com> writes:

> From e5e82efbf6f736984668cc5e94d62635ed4de05e Mon Sep 17 00:00:00 2001
> From: Teerth Reddy <teerth@ti.com>
> Date: Wed, 23 Dec 2009 18:40:34 +0530
> Subject: [PATCH] Dynamic Calculation of SDRC clock stabilization delay

Subject prefix should probably be 'OMAP3: SDRC:'.

> The patch has the changes to calculate the dpll3 clock
> stabilization delay dynamically. The SRAM delay is
> calibrated during bootup using the gptimers and used while
> calculating the stabilization delay. 

Thanks for the update using the GPtimer.  I think this simplifies
things quite a bit in terms of upstream code, however there are a
couple of new snags.  More below...

> By using the dynamic
> method the dependency on the type of cache being used is
> removed. Hence there is no need of loop based calculation.
>
> TO DO: However the value of 6us for SDRC to stabilize has
> been hardcoded currently. The formula used to calculate
> this value gives a very small number thus causing
> instability. The right formula will be used which will
> work across all boards.

Can you explain which platforms the hard-coded value should work for
and how that value was determined?

Also, should we be expecting another patch for this 'TO DO' part.
To me this is a problem that prevents merging this patch.

> Signed-off-by: Teerth Reddy <teerth@ti.com>
> Signed-off-by: Romit Dasgupta <romit@ti.com>
> ---
>  arch/arm/mach-omap2/clock34xx.c        |   21 +++++++++------
>  arch/arm/mach-omap2/clock34xx.h        |    2 +
>  arch/arm/mach-omap2/clock34xx_data.c   |    4 +++
>  arch/arm/mach-omap2/sram34xx.S         |   25 ++++++++++++++++++
>  arch/arm/plat-omap/include/plat/sram.h |    5 +++
>  arch/arm/plat-omap/sram.c              |   43 ++++++++++++++++++++++++++++++++
>  6 files changed, 91 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
> index d9329e2..6d87419 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -56,9 +56,18 @@
>   */
>  #define DPLL5_FREQ_FOR_USBHOST		120000000
>  
> +/*
> + * SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us
> + * TO DO: Use formula to calculate across all platforms
> + */
> +#define		SDRC_TIME_STABILIZE	6
> +
>  /* needed by omap3_core_dpll_m2_set_rate() */
>  struct clk *sdrc_ick_p, *arm_fck_p;
>  
> +/* SRAM delay required for sdrc clk stab delay calculation */
> +unsigned int delay_sram;
> +

Please get rid of the global variable and get the delay value
by calling a function that returns the value.  Also, please
be sure to do this with SMP in mind.

>  /**
>   * omap3430es2_clk_ssi_find_idlest - return CM_IDLEST info for SSI
>   * @clk: struct clk * being enabled
> @@ -215,16 +224,10 @@ int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
>  		unlock_dll = 1;
>  	}
>  
> -	/*
> -	 * XXX This only needs to be done when the CPU frequency changes
> -	 */
> +	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
> +
>  	_mpurate = arm_fck_p->rate / CYCLES_PER_MHZ;
> -	c = (_mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
> -	c += 1;  /* for safety */
> -	c *= SDRC_MPURATE_LOOPS;
> -	c >>= SDRC_MPURATE_SCALE;
> -	if (c == 0)
> -		c = 1;
> +	c = ((SDRC_TIME_STABILIZE * _mpurate) / (delay_sram * 2));
>  
>  	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
>  		 validrate);
> diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
> index 9a2c07e..1bc5044 100644
> --- a/arch/arm/mach-omap2/clock34xx.h
> +++ b/arch/arm/mach-omap2/clock34xx.h
> @@ -21,4 +21,6 @@ extern const struct clkops clkops_omap3430es2_hsotgusb_wait;
>  extern const struct clkops clkops_omap3430es2_dss_usbhost_wait;
>  extern const struct clkops clkops_noncore_dpll_ops;
>  
> +extern unsigned int delay_sram;
> +
>  #endif
> diff --git a/arch/arm/mach-omap2/clock34xx_data.c b/arch/arm/mach-omap2/clock34xx_data.c
> index 8bdcc9c..e5d0941 100644
> --- a/arch/arm/mach-omap2/clock34xx_data.c
> +++ b/arch/arm/mach-omap2/clock34xx_data.c
> @@ -22,6 +22,7 @@
>  
>  #include <plat/control.h>
>  #include <plat/clkdev_omap.h>
> +#include <plat/sram.h>
>  
>  #include "clock.h"
>  #include "clock34xx.h"
> @@ -3285,5 +3286,8 @@ int __init omap2_clk_init(void)
>  	sdrc_ick_p = clk_get(NULL, "sdrc_ick");
>  	arm_fck_p = clk_get(NULL, "arm_fck");
>  
> +	/* Measure sram delay */
> +	delay_sram = measure_sram_delay(10000);
> +	pr_debug("SRAM delay: %d\n", delay_sram);
>  	return 0;
>  }
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> index de99ba2..b4fba50 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -68,6 +68,10 @@
>  /* CM_CLKSEL1_PLL bit settings */
>  #define CORE_DPLL_CLKOUT_DIV_SHIFT	0x1b
>  
> +#define	GPTIMER10_TCRR_PHY			0x48086028
> +
> +#define	GPTIMER10_TCRR	OMAP2_L4_IO_ADDRESS(GPTIMER10_TCRR_PHY)
> +
>  /*
>   * omap3_sram_configure_core_dpll - change DPLL3 M2 divider
>   *
> @@ -313,3 +317,24 @@ core_m2_mask_val:
>  ENTRY(omap3_sram_configure_core_dpll_sz)
>  	.word	. - omap3_sram_configure_core_dpll
>  
> +ENTRY(__sram_wait_delay)
> +	stmfd	sp!, {r1-r12, lr}	@ store regs to stack
> + 	ldr	r2, gptimer10_counter
> +	ldr	r3, [r2]
> +
> +loop1:
> +	subs 	r0, r0, #1
> +	bne	loop1
> +
> +	isb
> +	ldr	r4, [r2]
> +	subs	r5, r4, r3
> +
> +	mov 	r0, r5 			@ return value
> +	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
> +
> +gptimer10_counter:
> +	.word	GPTIMER10_TCRR
> +
> +ENTRY(__sram_wait_delay_sz)
> +	.word	. - __sram_wait_delay
> diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
> index 16a1b45..7d0be2a 100644
> --- a/arch/arm/plat-omap/include/plat/sram.h
> +++ b/arch/arm/plat-omap/include/plat/sram.h
> @@ -69,6 +69,11 @@ extern u32 omap3_sram_configure_core_dpll(
>  			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
>  extern unsigned long omap3_sram_configure_core_dpll_sz;
>  
> +extern unsigned int measure_sram_delay(unsigned int);
> +
> +extern u32 __sram_wait_delay(unsigned int);
> +extern unsigned long __sram_wait_delay_sz;
> +
>  #ifdef CONFIG_PM
>  extern void omap_push_sram_idle(void);
>  #else
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index d8d5094..ddbdaaf 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -30,6 +30,9 @@
>  #include <plat/cpu.h>
>  #include <plat/vram.h>
>  
> +#include <linux/clk.h>
> +#include <plat/dmtimer.h>
> +
>  #include <plat/control.h>
>  
>  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> @@ -437,11 +440,51 @@ static inline int omap34xx_sram_init(void)
>  }
>  #endif
>  
> +
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned long (*_omap3_sram_delay)(unsigned long);
> +unsigned int  measure_sram_delay(unsigned int loop)
> +{
> +	static struct omap_dm_timer *gpt;
> +	unsigned long flags, diff = 0, gt_rate, mpurate;
> +	unsigned int delay_sram, error_gain;
> +
> +	omap_dm_timer_init();
> +	gpt = omap_dm_timer_request_specific(10);

While I understand the reasons you used a specific timer, I'm not
crazy about it.  Among other things, it has also forced you to
hard-code OMAP3 specific register values where this should be done
more generically.

The sram_delay function should take a struct dm_timer as an argument,
but there's currently no way to get the base address.  So, I see a
couple options.

Add a omap_dm_timer_get_phys_base() or similar to the DM timer API and
then pass that value into the sram delay function and use register
offsets in the asm function instead of fixed addresses.

That being said however, by default, the GPtimers are in posted mode,
so your reads are not taking that into account.

Ideally, what's needed is extending the DMtimer API to have a delay
function.  Something like: 

/* add kerneldoc description here */
unsigned int omap_dm_timer_delay_loops(struct omap_dm_timer *timer, u32 loops)
{
	unsigned int start, end;

	start = omap_dm_timer_read_reg(timer, OMAP_TIMER_COUNTER_REG);
        while(loops--)
                ;                
	end = omap_dm_timer_read_reg(timer, OMAP_TIMER_COUNTER_REG);

	return start - end;
}

which is then copied to SRAM to be executed.  This would possibly lead
to a few extra cycles counted because the stack variables and the
calls to the DMtimer API would in SDRAM, but the tightloop would be
running from SRAM.

Of course, I've been on vacation for two weeks, so I could be
forgetting some other basic reason this wouldn't work.  

> +	if (!gpt)
> +		pr_err("Could not get the gptimer\n");
> +	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> +
> +	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> +	omap_dm_timer_set_load_start(gpt, 0, 0);
> +
> +	local_irq_save(flags);
> +	diff = _omap3_sram_delay(loop);
> +	local_irq_restore(flags);
> +
> +	omap_dm_timer_stop(gpt);
> +	omap_dm_timer_free(gpt);
> +
> +	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));

Please some error checking when using clock API.

> +	/* calculate the sram delay */
> +	delay_sram = (((mpurate / gt_rate) * diff) / 20000);
> +
> +	error_gain = mpurate / gt_rate;
> +	delay_sram = delay_sram + error_gain;
> +
> +	return delay_sram;
> +}
> +#endif
> +
>  int __init omap_sram_init(void)
>  {
>  	omap_detect_sram();
>  	omap_map_sram();
>  
> +	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> +						__sram_wait_delay_sz);
> +
>  	if (!(cpu_class_is_omap2()))
>  		omap1_sram_init();
>  	else if (cpu_is_omap242x())

Kevin

Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Paul Walmsley]

Hi Teerth,

any update on Kevin's comments?

This patch shouldn't use a hardcoded value.  Please explain further why
that formula can't be included...

- Paul


On Wed, 6 Jan 2010, Kevin Hilman wrote:

> "Reddy, Teerth" <teerth@ti.com> writes:
> 
> > From e5e82efbf6f736984668cc5e94d62635ed4de05e Mon Sep 17 00:00:00 2001
> > From: Teerth Reddy <teerth@ti.com>
> > Date: Wed, 23 Dec 2009 18:40:34 +0530
> > Subject: [PATCH] Dynamic Calculation of SDRC clock stabilization delay
> 
> Subject prefix should probably be 'OMAP3: SDRC:'.
> 
> > The patch has the changes to calculate the dpll3 clock
> > stabilization delay dynamically. The SRAM delay is
> > calibrated during bootup using the gptimers and used while
> > calculating the stabilization delay. 
> 
> Thanks for the update using the GPtimer.  I think this simplifies
> things quite a bit in terms of upstream code, however there are a
> couple of new snags.  More below...
> 
> > By using the dynamic
> > method the dependency on the type of cache being used is
> > removed. Hence there is no need of loop based calculation.
> >
> > TO DO: However the value of 6us for SDRC to stabilize has
> > been hardcoded currently. The formula used to calculate
> > this value gives a very small number thus causing
> > instability. The right formula will be used which will
> > work across all boards.
> 
> Can you explain which platforms the hard-coded value should work for
> and how that value was determined?
> 
> Also, should we be expecting another patch for this 'TO DO' part.
> To me this is a problem that prevents merging this patch.
> 
> > Signed-off-by: Teerth Reddy <teerth@ti.com>
> > Signed-off-by: Romit Dasgupta <romit@ti.com>
> > ---
> >  arch/arm/mach-omap2/clock34xx.c        |   21 +++++++++------
> >  arch/arm/mach-omap2/clock34xx.h        |    2 +
> >  arch/arm/mach-omap2/clock34xx_data.c   |    4 +++
> >  arch/arm/mach-omap2/sram34xx.S         |   25 ++++++++++++++++++
> >  arch/arm/plat-omap/include/plat/sram.h |    5 +++
> >  arch/arm/plat-omap/sram.c              |   43 ++++++++++++++++++++++++++++++++
> >  6 files changed, 91 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
> > index d9329e2..6d87419 100644
> > --- a/arch/arm/mach-omap2/clock34xx.c
> > +++ b/arch/arm/mach-omap2/clock34xx.c
> > @@ -56,9 +56,18 @@
> >   */
> >  #define DPLL5_FREQ_FOR_USBHOST		120000000
> >  
> > +/*
> > + * SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us
> > + * TO DO: Use formula to calculate across all platforms
> > + */
> > +#define		SDRC_TIME_STABILIZE	6
> > +
> >  /* needed by omap3_core_dpll_m2_set_rate() */
> >  struct clk *sdrc_ick_p, *arm_fck_p;
> >  
> > +/* SRAM delay required for sdrc clk stab delay calculation */
> > +unsigned int delay_sram;
> > +
> 
> Please get rid of the global variable and get the delay value
> by calling a function that returns the value.  Also, please
> be sure to do this with SMP in mind.
> 
> >  /**
> >   * omap3430es2_clk_ssi_find_idlest - return CM_IDLEST info for SSI
> >   * @clk: struct clk * being enabled
> > @@ -215,16 +224,10 @@ int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
> >  		unlock_dll = 1;
> >  	}
> >  
> > -	/*
> > -	 * XXX This only needs to be done when the CPU frequency changes
> > -	 */
> > +	/* Calculate the number of MPU cycles to wait for SDRC to stabilize */
> > +
> >  	_mpurate = arm_fck_p->rate / CYCLES_PER_MHZ;
> > -	c = (_mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
> > -	c += 1;  /* for safety */
> > -	c *= SDRC_MPURATE_LOOPS;
> > -	c >>= SDRC_MPURATE_SCALE;
> > -	if (c == 0)
> > -		c = 1;
> > +	c = ((SDRC_TIME_STABILIZE * _mpurate) / (delay_sram * 2));
> >  
> >  	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
> >  		 validrate);
> > diff --git a/arch/arm/mach-omap2/clock34xx.h b/arch/arm/mach-omap2/clock34xx.h
> > index 9a2c07e..1bc5044 100644
> > --- a/arch/arm/mach-omap2/clock34xx.h
> > +++ b/arch/arm/mach-omap2/clock34xx.h
> > @@ -21,4 +21,6 @@ extern const struct clkops clkops_omap3430es2_hsotgusb_wait;
> >  extern const struct clkops clkops_omap3430es2_dss_usbhost_wait;
> >  extern const struct clkops clkops_noncore_dpll_ops;
> >  
> > +extern unsigned int delay_sram;
> > +
> >  #endif
> > diff --git a/arch/arm/mach-omap2/clock34xx_data.c b/arch/arm/mach-omap2/clock34xx_data.c
> > index 8bdcc9c..e5d0941 100644
> > --- a/arch/arm/mach-omap2/clock34xx_data.c
> > +++ b/arch/arm/mach-omap2/clock34xx_data.c
> > @@ -22,6 +22,7 @@
> >  
> >  #include <plat/control.h>
> >  #include <plat/clkdev_omap.h>
> > +#include <plat/sram.h>
> >  
> >  #include "clock.h"
> >  #include "clock34xx.h"
> > @@ -3285,5 +3286,8 @@ int __init omap2_clk_init(void)
> >  	sdrc_ick_p = clk_get(NULL, "sdrc_ick");
> >  	arm_fck_p = clk_get(NULL, "arm_fck");
> >  
> > +	/* Measure sram delay */
> > +	delay_sram = measure_sram_delay(10000);
> > +	pr_debug("SRAM delay: %d\n", delay_sram);
> >  	return 0;
> >  }
> > diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> > index de99ba2..b4fba50 100644
> > --- a/arch/arm/mach-omap2/sram34xx.S
> > +++ b/arch/arm/mach-omap2/sram34xx.S
> > @@ -68,6 +68,10 @@
> >  /* CM_CLKSEL1_PLL bit settings */
> >  #define CORE_DPLL_CLKOUT_DIV_SHIFT	0x1b
> >  
> > +#define	GPTIMER10_TCRR_PHY			0x48086028
> > +
> > +#define	GPTIMER10_TCRR	OMAP2_L4_IO_ADDRESS(GPTIMER10_TCRR_PHY)
> > +
> >  /*
> >   * omap3_sram_configure_core_dpll - change DPLL3 M2 divider
> >   *
> > @@ -313,3 +317,24 @@ core_m2_mask_val:
> >  ENTRY(omap3_sram_configure_core_dpll_sz)
> >  	.word	. - omap3_sram_configure_core_dpll
> >  
> > +ENTRY(__sram_wait_delay)
> > +	stmfd	sp!, {r1-r12, lr}	@ store regs to stack
> > + 	ldr	r2, gptimer10_counter
> > +	ldr	r3, [r2]
> > +
> > +loop1:
> > +	subs 	r0, r0, #1
> > +	bne	loop1
> > +
> > +	isb
> > +	ldr	r4, [r2]
> > +	subs	r5, r4, r3
> > +
> > +	mov 	r0, r5 			@ return value
> > +	ldmfd	sp!, {r1-r12, pc}	@ restore regs and return
> > +
> > +gptimer10_counter:
> > +	.word	GPTIMER10_TCRR
> > +
> > +ENTRY(__sram_wait_delay_sz)
> > +	.word	. - __sram_wait_delay
> > diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-omap/include/plat/sram.h
> > index 16a1b45..7d0be2a 100644
> > --- a/arch/arm/plat-omap/include/plat/sram.h
> > +++ b/arch/arm/plat-omap/include/plat/sram.h
> > @@ -69,6 +69,11 @@ extern u32 omap3_sram_configure_core_dpll(
> >  			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
> >  extern unsigned long omap3_sram_configure_core_dpll_sz;
> >  
> > +extern unsigned int measure_sram_delay(unsigned int);
> > +
> > +extern u32 __sram_wait_delay(unsigned int);
> > +extern unsigned long __sram_wait_delay_sz;
> > +
> >  #ifdef CONFIG_PM
> >  extern void omap_push_sram_idle(void);
> >  #else
> > diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> > index d8d5094..ddbdaaf 100644
> > --- a/arch/arm/plat-omap/sram.c
> > +++ b/arch/arm/plat-omap/sram.c
> > @@ -30,6 +30,9 @@
> >  #include <plat/cpu.h>
> >  #include <plat/vram.h>
> >  
> > +#include <linux/clk.h>
> > +#include <plat/dmtimer.h>
> > +
> >  #include <plat/control.h>
> >  
> >  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> > @@ -437,11 +440,51 @@ static inline int omap34xx_sram_init(void)
> >  }
> >  #endif
> >  
> > +
> > +#ifdef CONFIG_ARCH_OMAP3
> > +unsigned long (*_omap3_sram_delay)(unsigned long);
> > +unsigned int  measure_sram_delay(unsigned int loop)
> > +{
> > +	static struct omap_dm_timer *gpt;
> > +	unsigned long flags, diff = 0, gt_rate, mpurate;
> > +	unsigned int delay_sram, error_gain;
> > +
> > +	omap_dm_timer_init();
> > +	gpt = omap_dm_timer_request_specific(10);
> 
> While I understand the reasons you used a specific timer, I'm not
> crazy about it.  Among other things, it has also forced you to
> hard-code OMAP3 specific register values where this should be done
> more generically.
> 
> The sram_delay function should take a struct dm_timer as an argument,
> but there's currently no way to get the base address.  So, I see a
> couple options.
> 
> Add a omap_dm_timer_get_phys_base() or similar to the DM timer API and
> then pass that value into the sram delay function and use register
> offsets in the asm function instead of fixed addresses.
> 
> That being said however, by default, the GPtimers are in posted mode,
> so your reads are not taking that into account.
> 
> Ideally, what's needed is extending the DMtimer API to have a delay
> function.  Something like: 
> 
> /* add kerneldoc description here */
> unsigned int omap_dm_timer_delay_loops(struct omap_dm_timer *timer, u32 loops)
> {
> 	unsigned int start, end;
> 
> 	start = omap_dm_timer_read_reg(timer, OMAP_TIMER_COUNTER_REG);
>         while(loops--)
>                 ;                
> 	end = omap_dm_timer_read_reg(timer, OMAP_TIMER_COUNTER_REG);
> 
> 	return start - end;
> }
> 
> which is then copied to SRAM to be executed.  This would possibly lead
> to a few extra cycles counted because the stack variables and the
> calls to the DMtimer API would in SDRAM, but the tightloop would be
> running from SRAM.
> 
> Of course, I've been on vacation for two weeks, so I could be
> forgetting some other basic reason this wouldn't work.  
> 
> > +	if (!gpt)
> > +		pr_err("Could not get the gptimer\n");
> > +	omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> > +
> > +	gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> > +	omap_dm_timer_set_load_start(gpt, 0, 0);
> > +
> > +	local_irq_save(flags);
> > +	diff = _omap3_sram_delay(loop);
> > +	local_irq_restore(flags);
> > +
> > +	omap_dm_timer_stop(gpt);
> > +	omap_dm_timer_free(gpt);
> > +
> > +	mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
> 
> Please some error checking when using clock API.
> 
> > +	/* calculate the sram delay */
> > +	delay_sram = (((mpurate / gt_rate) * diff) / 20000);
> > +
> > +	error_gain = mpurate / gt_rate;
> > +	delay_sram = delay_sram + error_gain;
> > +
> > +	return delay_sram;
> > +}
> > +#endif
> > +
> >  int __init omap_sram_init(void)
> >  {
> >  	omap_detect_sram();
> >  	omap_map_sram();
> >  
> > +	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> > +						__sram_wait_delay_sz);
> > +
> >  	if (!(cpu_class_is_omap2()))
> >  		omap1_sram_init();
> >  	else if (cpu_is_omap242x())
> 
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


- Paul

RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Reddy, Teerth]

Hi Paul,

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Thursday, January 21, 2010 11:05 AM
> To: Reddy, Teerth
> Cc: Kevin Hilman; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock
> stabilization delay
> 
> Hi Teerth,
> 
> any update on Kevin's comments?
> 
> This patch shouldn't use a hardcoded value.  Please explain further why
> that formula can't be included...

Time required to switch clocks -
4 REFCLKS + 2 CLKOUTX2
Gives a small number which doesn't seem to work and causes instability during DVFS. I found that the value of 6us is safe and stable for the DVFS.I tried this value on OMAP3430 sdp, zoom2 and zoom3 and it worked fine.
I will let you know more about the formula once I get the updates.

Regards
Teerth

RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

[Paul Walmsley]

Hello Teerth,

On Thu, 21 Jan 2010, Reddy, Teerth wrote:

> > -----Original Message-----
> > From: Paul Walmsley [mailto:paul@pwsan.com]
> > Sent: Thursday, January 21, 2010 11:05 AM
> > To: Reddy, Teerth
> > Cc: Kevin Hilman; linux-omap@vger.kernel.org
> > Subject: Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock
> > stabilization delay
> > 
> > Hi Teerth,
> > 
> > any update on Kevin's comments?
> > 
> > This patch shouldn't use a hardcoded value.  Please explain further why
> > that formula can't be included...
> 
> Time required to switch clocks - 4 REFCLKS + 2 CLKOUTX2 Gives a small 
> number which doesn't seem to work and causes instability during DVFS. I 
> found that the value of 6us is safe and stable for the DVFS.I tried this 
> value on OMAP3430 sdp, zoom2 and zoom3 and it worked fine. I will let 
> you know more about the formula once I get the updates.

Any update on this?  Still waiting.  I don't see any reason why this 
formula couldn't be encoded.  If you can't get your implementation of 
the formula to work, please post the code for us to examine.  6 
microseconds is definitely too long for some cases and too short for 
others, so hardcoding that value is not correct.


- Paul