[PATCH v2] arm: da850: change ASYNC/PLL0_SYSCLK3 clock rate with DVFS

[PATCH v2] arm: da850: change ASYNC/PLL0_SYSCLK3 clock rate with DVFS

[Manjunathappa, Prakash]

Clock for EMIF is derived from ASYNC clock domain(PLL0_SYSCLK3) and was
configured with fixed divider as there was no significant performance
degradation with existing NAND/NOR EMIF devices if it is not
reconfigured accordingly at different OPPs.

On systems where devices other than NAND/NOR are interfaced through
EMIF, such performance degradation may not be desirable. So change the
PLL0_SYSCLK3 output frequency for different OPPs by re-configuring
the divider value.

Also add Kconfig option to support platforms requiring fixed EMIF clock
rate.

Configured values are as per rates specified in OMAP-L138 Data sheet
(http://www.ti.com/lit/ds/symlink/omap-l138.pdf, Table 5-5).

Patch addresses concerns raised in below thread of Linux-DaVinci-community:
http://www.mail-archive.com/davinci-linux-open-source at linux.davincidsp.com/msg22535.html

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
---
Since v1:
Have option to configure fixed EMIF clock rate via Kconfig option.

 arch/arm/mach-davinci/Kconfig                |    9 ++++++
 arch/arm/mach-davinci/board-da850-evm.c      |   22 ++++++++++++++++
 arch/arm/mach-davinci/cpufreq.c              |    2 +-
 arch/arm/mach-davinci/da850.c                |   36 ++++++++++++++++++++++++-
 arch/arm/mach-davinci/include/mach/cpufreq.h |    1 +
 5 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/Kconfig b/arch/arm/mach-davinci/Kconfig
index 32d837d..0a2fb6c 100644
--- a/arch/arm/mach-davinci/Kconfig
+++ b/arch/arm/mach-davinci/Kconfig
@@ -192,6 +192,15 @@ config DA850_UI_RMII
 
 endchoice
 
+config DA850_FIX_PLL0_SYSCLK3RATE
+	int "PLL0_SYSCLK3 rate"
+	depends on ARCH_DAVINCI_DA850
+	range 0 148000000
+	default 0
+	help
+	  Set this value with desired EMIF clock rate, Otherwise EMIF clock
+          rate is decided based on OPP value.
+
 config DA850_WL12XX
 	bool "AM18x wl1271 daughter board"
 	depends on MACH_DAVINCI_DA850_EVM
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index a70de24..1fb3050 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -13,6 +13,7 @@
  */
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/clk.h>
 #include <linux/console.h>
 #include <linux/i2c.h>
 #include <linux/i2c/at24.h>
@@ -1253,6 +1254,17 @@ static __init int da850_wl12xx_init(void)
 
 #endif /* CONFIG_DA850_WL12XX */
 
+static __init int da850_set_emif_clk_rate(void)
+{
+	struct clk *emif_clk;
+
+	emif_clk = clk_get(NULL, "pll0_sysclk3");
+	if (WARN(IS_ERR(emif_clk), "Unable to get emif clock\n"))
+		return PTR_ERR(emif_clk);
+
+	return clk_set_rate(emif_clk, CONFIG_DA850_FIX_PLL0_SYSCLK3RATE);
+}
+
 #define DA850EVM_SATA_REFCLKPN_RATE	(100 * 1000 * 1000)
 
 static __init void da850_evm_init(void)
@@ -1264,6 +1276,16 @@ static __init void da850_evm_init(void)
 		pr_warning("da850_evm_init: TPS65070 PMIC init failed: %d\n",
 				ret);
 
+	/*
+	 * Though bootloader takes care to set emif clock at allowed
+	 * possible rate. Kernel needs to reconfigure this rate to
+	 * support platforms requiring fixed emif clock rate.
+	 */
+	ret = da850_set_emif_clk_rate();
+	if (ret)
+		pr_warning("da850_evm_init: Failed to set rate of pll0_sysclk3/emif clock: %d\n",
+				ret);
+
 	ret = da850_register_edma(da850_edma_rsv);
 	if (ret)
 		pr_warning("da850_evm_init: edma registration failed: %d\n",
diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index 031048f..1f14cbe 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -216,7 +216,7 @@ static int __init davinci_cpufreq_probe(struct platform_device *pdev)
 	asyncclk = clk_get(cpufreq.dev, "async");
 	if (!IS_ERR(asyncclk)) {
 		cpufreq.asyncclk = asyncclk;
-		cpufreq.asyncrate = clk_get_rate(asyncclk);
+		cpufreq.asyncrate = pdata->emif_rate;
 	}
 
 	return cpufreq_register_driver(&davinci_driver);
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index b44dc84..692203c 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -49,6 +49,7 @@
 static int da850_set_armrate(struct clk *clk, unsigned long rate);
 static int da850_round_armrate(struct clk *clk, unsigned long rate);
 static int da850_set_pll0rate(struct clk *clk, unsigned long armrate);
+static int da850_set_pll0sysclk3_rate(struct clk *clk, unsigned long rate);
 
 static struct pll_data pll0_data = {
 	.num		= 1,
@@ -88,8 +89,8 @@ static struct clk pll0_sysclk3 = {
 	.parent		= &pll0_clk,
 	.flags		= CLK_PLL,
 	.div_reg	= PLLDIV3,
-	.set_rate	= davinci_set_sysclk_rate,
-	.maxrate	= 100000000,
+	.set_rate	= da850_set_pll0sysclk3_rate,
+	.maxrate	= 148000000,
 };
 
 static struct clk pll0_sysclk4 = {
@@ -799,6 +800,36 @@ static void da850_set_async3_src(int pllnum)
 	__raw_writel(v, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG));
 }
 
+static int da850_set_pll0sysclk3_rate(struct clk *clk, unsigned long rate)
+{
+	struct clk *arm_clk;
+	unsigned long sys_clk3_rate = 148000000;
+	int ret;
+
+	arm_clk = clk_get(NULL, "arm");
+	if (WARN(IS_ERR(arm_clk), "Unable to get ARM clock\n"))
+		return PTR_ERR(arm_clk);
+
+	/* Set EMIF clock based on OPPs */
+	switch (clk_get_rate(arm_clk)) {
+	case 200000000:
+		sys_clk3_rate = 75000000;
+		break;
+	case 96000000:
+		sys_clk3_rate = 50000000;
+		break;
+	}
+
+	if (rate)
+		sys_clk3_rate = min(sys_clk3_rate, rate);
+
+	ret = davinci_set_sysclk_rate(clk, sys_clk3_rate);
+	if (WARN_ON(ret))
+		return ret;
+
+	return 0;
+}
+
 #ifdef CONFIG_CPU_FREQ
 /*
  * Notes:
@@ -900,6 +931,7 @@ static struct davinci_cpufreq_config cpufreq_info = {
 	.init = da850_regulator_init,
 	.set_voltage = da850_set_voltage,
 #endif
+	.emif_rate = CONFIG_DA850_FIX_PLL0_SYSCLK3RATE,
 };
 
 #ifdef CONFIG_REGULATOR
diff --git a/arch/arm/mach-davinci/include/mach/cpufreq.h b/arch/arm/mach-davinci/include/mach/cpufreq.h
index 3c089cf..3f58b71 100644
--- a/arch/arm/mach-davinci/include/mach/cpufreq.h
+++ b/arch/arm/mach-davinci/include/mach/cpufreq.h
@@ -21,6 +21,7 @@ struct davinci_cpufreq_config {
 	struct cpufreq_frequency_table *freq_table;
 	int (*set_voltage) (unsigned int index);
 	int (*init) (void);
+	unsigned int emif_rate;
 };
 
 #endif
-- 
1.7.1

[PATCH v2] arm: da850: change ASYNC/PLL0_SYSCLK3 clock rate with DVFS

[Sekhar Nori]

Hi Prakash,

On 4/5/2012 2:43 PM, Manjunathappa, Prakash wrote:
> Clock for EMIF is derived from ASYNC clock domain(PLL0_SYSCLK3) and was
> configured with fixed divider as there was no significant performance
> degradation with existing NAND/NOR EMIF devices if it is not
> reconfigured accordingly at different OPPs.

This is not correct AFAICT. The divider is not fixed in current code.
There is an attempt to adjust the async1 (PLL0 SYSCLK3 rate) and keep it
constant at 100MHz by adjusting the divider at each OPP transition (see
the call to clock_set_rate() on async clock in cpufreq.c). This was
based on an earlier understanding that PLL0_SYSCLK3 can support 100MHz
at all OPPs and all AEMIF modes. Looking at the datasheet now, it is
clear that that assumption is not true.

> 
> On systems where devices other than NAND/NOR are interfaced through
> EMIF, such performance degradation may not be desirable. So change the
> PLL0_SYSCLK3 output frequency for different OPPs by re-configuring
> the divider value.

This again is not the point. The issue is that depending on what mode
you have AEMIF running in *and* depending on what OPP you are at, a
particular value of ASYNC1 clock is required to get maximum performance
out of AEMIF.

> 
> Also add Kconfig option to support platforms requiring fixed EMIF clock
> rate.

This breaks single image support for all DA850 boards. Kconfig in this
case is not an option.

Why not make the OPP table board specific while maintain a sane (and
safe) default in SoC file so there is something to fall back upon in
case a board does not need to define its own? This should also address
Christian's concern in the other e-mail thread about board specific OPP
values being hardcoded in SoC file with no way to override them.

Thanks,
Sekhar

[PATCH v2] arm: da850: change ASYNC/PLL0_SYSCLK3 clock rate with DVFS

[Manjunathappa, Prakash]

Hi Sekhar,

On Tue, Apr 10, 2012 at 00:35:57, Nori, Sekhar wrote:
> Hi Prakash,
> 
> On 4/5/2012 2:43 PM, Manjunathappa, Prakash wrote:
> > Clock for EMIF is derived from ASYNC clock domain(PLL0_SYSCLK3) and was
> > configured with fixed divider as there was no significant performance
> > degradation with existing NAND/NOR EMIF devices if it is not
> > reconfigured accordingly at different OPPs.
> 
> This is not correct AFAICT. The divider is not fixed in current code.
> There is an attempt to adjust the async1 (PLL0 SYSCLK3 rate) and keep it
> constant at 100MHz by adjusting the divider at each OPP transition (see
> the call to clock_set_rate() on async clock in cpufreq.c). This was
> based on an earlier understanding that PLL0_SYSCLK3 can support 100MHz
> at all OPPs and all AEMIF modes. Looking at the datasheet now, it is
> clear that that assumption is not true.
> 

I will correct this.
 
> > 
> > On systems where devices other than NAND/NOR are interfaced through
> > EMIF, such performance degradation may not be desirable. So change the
> > PLL0_SYSCLK3 output frequency for different OPPs by re-configuring
> > the divider value.
> 
> This again is not the point. The issue is that depending on what mode
> you have AEMIF running in *and* depending on what OPP you are at, a
> particular value of ASYNC1 clock is required to get maximum performance
> out of AEMIF.
> 

Correct, I will reword it to convey the same.

> > 
> > Also add Kconfig option to support platforms requiring fixed EMIF clock
> > rate.
> 
> This breaks single image support for all DA850 boards. Kconfig in this
> case is not an option.
> 
> Why not make the OPP table board specific while maintain a sane (and
> safe) default in SoC file so there is something to fall back upon in
> case a board does not need to define its own? This should also address
> Christian's concern in the other e-mail thread about board specific OPP
> values being hardcoded in SoC file with no way to override them.
> 

Ok, I will look at this option and try to implement. 

Thanks,
Prakash