[PATCH 0/5] ARM: S5PV210: CPUFREQ Initial Support

[PATCH 0/5] ARM: S5PV210: CPUFREQ Initial Support

[MyungJoo Ham]

S5PV210 CPUFREQ Initial Support.

This is a series of patches to enable CPUFREQ for S5PV210.

Although this works without PMIC's DVS support, it is not
as effective without DVS support as supposed. AVS is not
supported in this version.



MyungJoo Ham (5):
  ARM: Samsung SoC: added hclk/pclk info to s3c_freq for s5pv210
    cpu-freq
  ARM: S5P: Added default pll values for APLL 800/1000MHz
  ARM: S5PV210: macros for clock registers at regs-clock.h
  ARM: S5PV210: add DMCx access virtual address
  ARM: S5PV210: Initial CPUFREQ Support

 arch/arm/Kconfig                                |    1 +
 arch/arm/mach-s5pv210/Makefile                  |    3 +
 arch/arm/mach-s5pv210/cpu.c                     |   12 +-
 arch/arm/mach-s5pv210/cpufreq-s5pv210.c         |  824 +++++++++++++++++++++++
 arch/arm/mach-s5pv210/include/mach/cpu-freq.h   |   50 ++
 arch/arm/mach-s5pv210/include/mach/map.h        |    8 +
 arch/arm/mach-s5pv210/include/mach/regs-clock.h |   45 ++-
 arch/arm/plat-s5p/include/plat/pll.h            |    8 +
 arch/arm/plat-samsung/include/plat/cpu-freq.h   |    6 +
 9 files changed, 953 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/mach-s5pv210/cpufreq-s5pv210.c
 create mode 100644 arch/arm/mach-s5pv210/include/mach/cpu-freq.h

[PATCH 1/5] ARM: Samsung SoC: added hclk/pclk info to s3c_freq for s5pv210 cpu-freq

[MyungJoo Ham]

S5PV210 requires msys/dsys info as well; thus, we've included those at
struct s3c_freq, which is used by CPUFREQ of S5PV210.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-samsung/include/plat/cpu-freq.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/cpu-freq.h b/arch/arm/plat-samsung/include/plat/cpu-freq.h
index 80c4a80..9e97df7 100644
--- a/arch/arm/plat-samsung/include/plat/cpu-freq.h
+++ b/arch/arm/plat-samsung/include/plat/cpu-freq.h
@@ -38,6 +38,12 @@ struct s3c_freq {
 	unsigned long	hclk_tns;	/* in 10ths of ns */
 	unsigned long	hclk;
 	unsigned long	pclk;
+#ifdef CONFIG_ARCH_S5PV210
+	unsigned long	hclk_msys;
+	unsigned long	pclk_msys;
+	unsigned long	hclk_dsys;
+	unsigned long	pclk_dsys;
+#endif
 };
 
 /**
-- 
1.6.3.3

[PATCH 2/5] ARM: S5P: Added default pll values for APLL 800/1000MHz

[MyungJoo Ham]

CPUFREQ of S5PV210 uses different APLL settings and we provide
such values for CPUFREQ at pll.h. We have been using differently
between EVT0 and EVT1 machines. Although this version of kernel
assumes that the CPU is EVT1, users may use code for EVT0 later.

Note that at 1GHz of ARMCLK, APLL should be 1GHz and for other lower
ARMCLK, APLL should be 800MHz.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/plat-s5p/include/plat/pll.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-s5p/include/plat/pll.h b/arch/arm/plat-s5p/include/plat/pll.h
index 7db3227..3112aba 100644
--- a/arch/arm/plat-s5p/include/plat/pll.h
+++ b/arch/arm/plat-s5p/include/plat/pll.h
@@ -21,6 +21,14 @@
 
 #include <asm/div64.h>
 
+#ifdef CONFIG_CPU_S5PC110_EVT0_ERRATA
+#define PLL45XX_APLL_VAL_1000	(1 << 31) | (0xfa<<16) | (0x6<<8) | (0x1)
+#define PLL45XX_APLL_VAL_800	(1 << 31) | (0xc8<<16) | (0x6<<8) | (0x1)
+#else
+#define PLL45XX_APLL_VAL_1000	(1 << 31) | (125<<16) | (3<<8) | (1)
+#define PLL45XX_APLL_VAL_800	(1 << 31) | (100<<16) | (3<<8) | (1)
+#endif
+
 enum pll45xx_type_t {
 	pll_4500,
 	pll_4502,
-- 
1.6.3.3

[PATCH 3/5] ARM: S5PV210: macros for clock registers at regs-clock.h

[MyungJoo Ham]

Previously, most of CLK_DIV/SRC register accessing mask and shift
values were used at arch/arm/mach-s5pv210/clock.c only; thus we
had not been using macros for these. However, as CPUFREQ uses
those shift and mask values as well, we'd better define them at a single
location, whose proper location would be regs-clock.h.

Note that only the information about registers used by CPUFREQ are
defined. However, we may need to define other registers later if we add
other parts.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-s5pv210/include/mach/regs-clock.h |   45 +++++++++++++++++++++--
 1 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-s5pv210/include/mach/regs-clock.h b/arch/arm/mach-s5pv210/include/mach/regs-clock.h
index 2a25ab4..a117ecc 100644
--- a/arch/arm/mach-s5pv210/include/mach/regs-clock.h
+++ b/arch/arm/mach-s5pv210/include/mach/regs-clock.h
@@ -66,11 +66,30 @@
 #define S5P_CLKGATE_BUS0	S5P_CLKREG(0x484)
 #define S5P_CLKGATE_BUS1	S5P_CLKREG(0x488)
 #define S5P_CLK_OUT		S5P_CLKREG(0x500)
+#define S5P_CLK_DIV_STAT0   S5P_CLKREG(0x1000)
+#define S5P_CLK_DIV_STAT1   S5P_CLKREG(0x1004)
+#define S5P_CLK_MUX_STAT0   S5P_CLKREG(0x1100)
+#define S5P_CLK_MUX_STAT1   S5P_CLKREG(0x1104)
+
+#define S5P_ARM_MCS_CON		S5P_CLKREG(0x6100)
 
 /* CLKSRC0 */
-#define S5P_CLKSRC0_MUX200_MASK		(0x1<<16)
-#define S5P_CLKSRC0_MUX166_MASK		(0x1<<20)
-#define S5P_CLKSRC0_MUX133_MASK		(0x1<<24)
+#define S5P_CLKSRC0_APLL_MASK		(0x1 << 0)
+#define S5P_CLKSRC0_APLL_SHIFT		(0)
+#define S5P_CLKSRC0_MPLL_MASK		(0x1 << 4)
+#define S5P_CLKSRC0_MPLL_SHIFT		(4)
+#define S5P_CLKSRC0_EPLL_MASK		(0x1 << 8)
+#define S5P_CLKSRC0_EPLL_SHIFT		(8)
+#define S5P_CLKSRC0_VPLL_MASK		(0x1 << 12)
+#define S5P_CLKSRC0_VPLL_SHIFT		(12)
+#define S5P_CLKSRC0_MUX200_MASK		(0x1 << 16)
+#define S5P_CLKSRC0_MUX200_SHIFT	(16)
+#define S5P_CLKSRC0_MUX166_MASK		(0x1 << 20)
+#define S5P_CLKSRC0_MUX166_SHIFT	(20)
+#define S5P_CLKSRC0_MUX133_MASK		(0x1 << 24)
+#define S5P_CLKSRC0_MUX133_SHIFT	(24)
+#define S5P_CLKSRC0_ONENAND_MASK	(0x1 << 28)
+#define S5P_CLKSRC0_ONENAND_SHIFT	(28)
 
 /* CLKDIV0 */
 #define S5P_CLKDIV0_APLL_SHIFT		(0)
@@ -90,6 +109,26 @@
 #define S5P_CLKDIV0_PCLK66_SHIFT	(28)
 #define S5P_CLKDIV0_PCLK66_MASK		(0x7 << S5P_CLKDIV0_PCLK66_SHIFT)
 
+/* CLKSRC2 */
+#define S5P_CLKSRC2_G3D_MASK		(0x3 << 0)
+#define S5P_CLKSRC2_G3D_SHIFT		(0)
+#define S5P_CLKSRC2_MFC_MASK		(0x3 << 4)
+#define S5P_CLKSRC2_MFC_SHIFT		(4)
+#define S5P_CLKSRC2_G2D_MASK		(0x3 << 8)
+#define S5P_CLKSRC2_G2D_SHIFT		(8)
+
+/* CLKDIV2 */
+#define S5P_CLKDIV2_G3D_MASK		(0xF << 0)
+#define S5P_CLKDIV2_G3D_SHIFT		(0)
+#define S5P_CLKDIV2_MFC_MASK		(0xF << 4)
+#define S5P_CLKDIV2_MFC_SHIFT		(4)
+#define S5P_CLKDIV2_G2D_MASK		(0xF << 8)
+#define S5P_CLKDIV2_G2D_SHIFT		(8)
+
+/* CLKDIV6 */
+#define S5P_CLKDIV6_ONEDRAM_MASK	(0xf<<28)
+#define S5P_CLKDIV6_ONEDRAM_SHIFT	(28)
+
 /* Registers related to power management */
 #define S5P_PWR_CFG		S5P_CLKREG(0xC000)
 #define S5P_EINT_WAKEUP_MASK	S5P_CLKREG(0xC004)
-- 
1.6.3.3

[PATCH 4/5] ARM: S5PV210: add DMCx access virtual address

[MyungJoo Ham]

This patch prepares CPUFREQ support for S5PV210 by adding definitions
for S5P_VA_DMCx accessed by CPUFREQ, which were not defined previously.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-s5pv210/cpu.c              |   12 +++++++++++-
 arch/arm/mach-s5pv210/include/mach/map.h |    8 ++++++++
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-s5pv210/cpu.c b/arch/arm/mach-s5pv210/cpu.c
index 94c632b..84a6708 100644
--- a/arch/arm/mach-s5pv210/cpu.c
+++ b/arch/arm/mach-s5pv210/cpu.c
@@ -59,7 +59,17 @@ static struct map_desc s5pv210_iodesc[] __initdata = {
 		.pfn		= __phys_to_pfn(S5PV210_PA_SROMC),
 		.length		= SZ_4K,
 		.type		= MT_DEVICE,
-	}
+	}, {
+		.virtual    = (unsigned long)S5P_VA_DMC0,
+		.pfn        = __phys_to_pfn(S5P_PA_DMC0),
+		.length     = SZ_4K,
+		.type       = MT_DEVICE,
+	}, {
+		.virtual    = (unsigned long)S5P_VA_DMC1,
+		.pfn        = __phys_to_pfn(S5P_PA_DMC1),
+		.length     = SZ_4K,
+		.type       = MT_DEVICE,
+	},
 };
 
 static void s5pv210_idle(void)
diff --git a/arch/arm/mach-s5pv210/include/mach/map.h b/arch/arm/mach-s5pv210/include/mach/map.h
index 17687f0..4945674 100644
--- a/arch/arm/mach-s5pv210/include/mach/map.h
+++ b/arch/arm/mach-s5pv210/include/mach/map.h
@@ -108,4 +108,12 @@
 #define SAMSUNG_PA_ADC		S5PV210_PA_ADC
 #define SAMSUNG_PA_KEYPAD	S5PV210_PA_KEYPAD
 
+/* DMC */
+#define S5P_PA_DMC0     (0xF0000000)
+#define S5P_VA_DMC0     S3C_ADDR(0x00a00000)
+
+#define S5P_PA_DMC1     (0xF1400000)
+#define S5P_VA_DMC1     S3C_ADDR(0x00b00000)
+
+
 #endif /* __ASM_ARCH_MAP_H */
-- 
1.6.3.3

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[MyungJoo Ham]

S5PV210 CPUFREQ Support.

This CPUFREQ may work without PMIC's DVS support. However, it is not
as effective without DVS support as supposed. AVS is not supported in
this version.

Note that CLK_SRC of some clocks including ARMCLK, G3D, G2D, MFC,
and ONEDRAM are modified directly without updating clksrc_src
representations of the clocks.  Because CPUFREQ reverts the CLK_SRC
to the supposed values, this does not affect the consistency as long as
there are no other modules that modifies clock sources of ARMCLK, G3D,
G2D, MFC, and ONEDRAM (and only CPUFREQ should have the control). As
soon as clock framework is settled, we may update source clocks
(.parent field) of those clocks accordingly later.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/Kconfig                              |    1 +
 arch/arm/mach-s5pv210/Makefile                |    3 +
 arch/arm/mach-s5pv210/cpufreq-s5pv210.c       |  824 +++++++++++++++++++++++++
 arch/arm/mach-s5pv210/include/mach/cpu-freq.h |   50 ++
 4 files changed, 878 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv210/cpufreq-s5pv210.c
 create mode 100644 arch/arm/mach-s5pv210/include/mach/cpu-freq.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 98922f7..d5a5916 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -701,6 +701,7 @@ config ARCH_S5PV210
 	select HAVE_CLK
 	select ARM_L1_CACHE_SHIFT_6
 	select ARCH_USES_GETTIMEOFFSET
+	select ARCH_HAS_CPUFREQ
 	help
 	  Samsung S5PV210/S5PC110 series based systems
 
diff --git a/arch/arm/mach-s5pv210/Makefile b/arch/arm/mach-s5pv210/Makefile
index aae592a..293dbac 100644
--- a/arch/arm/mach-s5pv210/Makefile
+++ b/arch/arm/mach-s5pv210/Makefile
@@ -34,3 +34,6 @@ obj-$(CONFIG_S5PV210_SETUP_I2C2) 	+= setup-i2c2.o
 obj-$(CONFIG_S5PV210_SETUP_KEYPAD)	+= setup-keypad.o
 obj-$(CONFIG_S5PV210_SETUP_SDHCI)       += setup-sdhci.o
 obj-$(CONFIG_S5PV210_SETUP_SDHCI_GPIO)	+= setup-sdhci-gpio.o
+
+# CPUFREQ (DVFS)
+obj-$(CONFIG_CPU_FREQ)		+= cpufreq-s5pv210.o
diff --git a/arch/arm/mach-s5pv210/cpufreq-s5pv210.c b/arch/arm/mach-s5pv210/cpufreq-s5pv210.c
new file mode 100644
index 0000000..d70ef3e
--- /dev/null
+++ b/arch/arm/mach-s5pv210/cpufreq-s5pv210.c
@@ -0,0 +1,824 @@
+/*  linux/arch/arm/plat-s5pc11x/s5pc11x-cpufreq.c
+ *
+ *  Copyright (C) 2010 Samsung Electronics Co., Ltd.
+ *
+ *  CPU frequency scaling for S5PC110
+ *  Based on cpu-sa1110.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/regulator/consumer.h>
+#include <linux/gpio.h>
+#include <asm/system.h>
+
+#include <mach/hardware.h>
+#include <mach/map.h>
+#include <mach/regs-clock.h>
+#include <mach/regs-gpio.h>
+#include <mach/cpu-freq.h>
+
+#include <plat/cpu-freq.h>
+#include <plat/pll.h>
+#include <plat/clock.h>
+#include <plat/gpio-cfg.h>
+#include <plat/regs-fb.h>
+#ifdef CONFIG_PM
+#include <plat/pm.h>
+#endif
+
+static struct clk *mpu_clk;
+static struct regulator *arm_regulator;
+static struct regulator *internal_regulator;
+
+struct s3c_cpufreq_freqs s3c_freqs;
+
+#ifdef CONFIG_CPU_S5PC110_EVT0_ERRATA
+#define	CPUFREQ_DISABLE_1GHZ
+#endif
+/* #define	CPUFREQ_DISABLE_100MHZ */
+
+/* Based on MAX8998 default RAMP time: 10mV/us */
+#define PMIC_RAMP_UP		(10)
+static unsigned long previous_arm_volt;
+
+/* If you don't need to wait exact ramp up delay,
+ * you can use the fixed delay time.
+ **/
+/* #define USE_FIXED_RAMP_UP_DELAY */
+
+/* frequency */
+static struct cpufreq_frequency_table s5pv210_freq_table[] = {
+	{L0, 1000*1000},
+	{L1, 800*1000},
+	{L2, 400*1000},
+	{L3, 200*1000},
+	{L4, 100*1000},
+	{0, CPUFREQ_TABLE_END},
+};
+
+struct s5pc11x_dvs_conf {
+	const unsigned long lvl;        /* DVFS level : L0,L1,L2,L3... */
+	unsigned long       arm_volt;   /* uV */
+	unsigned long       int_volt;   /* uV */
+};
+
+static struct s5pc11x_dvs_conf s5pv210_dvs_conf[] = {
+#ifdef CONFIG_CPU_S5PC110_EVT0_ERRATA
+	{
+		.lvl        = L0,
+		.arm_volt   = 1300000,
+		.int_volt   = 1200000,
+	}, {
+		.lvl        = L1,
+		.arm_volt   = 1250000,
+		.int_volt   = 1200000,
+
+	}, {
+		.lvl        = L2,
+		.arm_volt   = 1250000,
+		.int_volt   = 1200000,
+
+	}, {
+		.lvl        = L3,
+		.arm_volt   = 1250000,
+		.int_volt   = 1200000,
+	}, {
+		.lvl        = L4,
+		.arm_volt   = 1250000,
+		.int_volt   = 1200000,
+	},
+#else
+	{
+		.lvl        = L0,
+		.arm_volt   = 1250000,
+		.int_volt   = 1100000,
+	}, {
+		.lvl        = L1,
+		.arm_volt   = 1200000,
+		.int_volt   = 1100000,
+
+	}, {
+		.lvl        = L2,
+		.arm_volt   = 1050000,
+		.int_volt   = 1100000,
+
+	}, {
+		.lvl        = L3,
+		.arm_volt   = 950000,
+		.int_volt   = 1100000,
+	}, {
+		.lvl        = L4,
+		.arm_volt   = 950000,
+		.int_volt   = 1000000,
+	},
+#endif
+};
+
+static u32 clkdiv_val[5][11] = {
+	/*{ APLL, A2M, HCLK_MSYS, PCLK_MSYS,
+	 * HCLK_DSYS, PCLK_DSYS, HCLK_PSYS, PCLK_PSYS, ONEDRAM,
+	 * MFC, G3D }
+	 */
+	/* L0 : [1000/200/200/100][166/83][133/66][200/200] */
+	{0, 4, 4, 1, 3, 1, 4, 1, 3, 0, 0},
+	/* L1 : [800/200/200/100][166/83][133/66][200/200] */
+	{0, 3, 3, 1, 3, 1, 4, 1, 3, 0, 0},
+	/* L2 : [400/200/200/100][166/83][133/66][200/200] */
+	{1, 3, 1, 1, 3, 1, 4, 1, 3, 0, 0},
+	/* L3 : [200/200/200/100][166/83][133/66][200/200] */
+	{3, 3, 0, 1, 3, 1, 4, 1, 3, 0, 0},
+	/* L4 : [100/100/100/100][83/83][66/66][100/100] */
+	{7, 7, 0, 0, 7, 0, 9, 0, 7, 0, 0},
+};
+
+static struct s3c_freq s5pv210_clk_info[] = {
+	[L0] = {	/* L0: 1GHz */
+		.fclk       = 1000000,
+		.armclk     = 1000000,
+		.hclk_tns   = 0,
+		.hclk       = 133000,
+		.pclk       = 66000,
+		.hclk_msys  = 200000,
+		.pclk_msys  = 100000,
+		.hclk_dsys  = 166750,
+		.pclk_dsys  = 83375,
+	},
+	[L1] = {	/* L1: 800MHz */
+		.fclk       = 800000,
+		.armclk     = 800000,
+		.hclk_tns   = 0,
+		.hclk       = 133000,
+		.pclk       = 66000,
+		.hclk_msys  = 200000,
+		.pclk_msys  = 100000,
+		.hclk_dsys  = 166750,
+		.pclk_dsys  = 83375,
+	},
+	[L2] = {	/* L2: 400MHz */
+		.fclk       = 800000,
+		.armclk     = 400000,
+		.hclk_tns   = 0,
+		.hclk       = 133000,
+		.pclk       = 66000,
+		.hclk_msys  = 200000,
+		.pclk_msys  = 100000,
+		.hclk_dsys  = 166750,
+		.pclk_dsys  = 83375,
+	},
+	[L3] = {	/* L3: 200MHz */
+		.fclk       = 800000,
+		.armclk     = 200000,
+		.hclk_tns   = 0,
+		.hclk       = 133000,
+		.pclk       = 66000,
+		.hclk_msys  = 200000,
+		.pclk_msys  = 100000,
+		.hclk_dsys  = 166750,
+		.pclk_dsys  = 83375,
+	},
+	[L4] = {	/* L4: 100MHz */
+		.fclk       = 800000,
+		.armclk     = 100000,
+		.hclk_tns   = 0,
+		.hclk       = 66000,
+		.pclk       = 66000,
+		.hclk_msys  = 100000,
+		.pclk_msys  = 100000,
+		.hclk_dsys  = 83375,
+		.pclk_dsys  = 83375,
+	},
+};
+
+int s5pv210_verify_speed(struct cpufreq_policy *policy)
+{
+
+	if (policy->cpu)
+		return -EINVAL;
+
+	return cpufreq_frequency_table_verify(policy, s5pv210_freq_table);
+}
+
+unsigned int s5pv210_getspeed(unsigned int cpu)
+{
+	unsigned long rate;
+
+	if (cpu)
+		return 0;
+
+	rate = clk_get_rate(mpu_clk) / KHZ_T;
+
+	return rate;
+}
+
+static void s5pv210_target_APLL2MPLL(unsigned int index,
+		unsigned int bus_speed_changing)
+{
+	unsigned int reg;
+
+	/* 1. Temporarily change divider for MFC and G3D
+	 * SCLKA2M(200/1=200)->(200/4=50)MHz
+	 **/
+	reg = __raw_readl(S5P_CLK_DIV2);
+	reg &= ~(S5P_CLKDIV2_G3D_MASK | S5P_CLKDIV2_MFC_MASK);
+	reg |= (0x3 << S5P_CLKDIV2_G3D_SHIFT) |
+		(0x3 << S5P_CLKDIV2_MFC_SHIFT);
+#ifndef CONFIG_CPU_S5PC110_EVT0_ERRATA
+	reg &= ~S5P_CLKDIV2_G2D_MASK;
+	reg |= (0x2 << S5P_CLKDIV2_G2D_SHIFT);
+#endif
+	__raw_writel(reg, S5P_CLK_DIV2);
+
+	/* For MFC, G3D dividing */
+	do {
+		reg = __raw_readl(S5P_CLK_DIV_STAT0);
+	} while (reg & ((1 << 16) | (1 << 17)));
+
+	/* 2. Change SCLKA2M(200MHz) to SCLKMPLL in MFC_MUX, G3D MUX
+	 * (100/4=50)->(667/4=166)MHz
+	 **/
+	reg = __raw_readl(S5P_CLK_SRC2);
+	reg &= ~(S5P_CLKSRC2_G3D_MASK | S5P_CLKSRC2_MFC_MASK);
+	reg |= (0x1 << S5P_CLKSRC2_G3D_SHIFT) |
+		(0x1 << S5P_CLKSRC2_MFC_SHIFT);
+#ifndef CONFIG_CPU_S5PC110_EVT0_ERRATA
+	reg &= ~S5P_CLKSRC2_G2D_MASK;
+	reg |= (0x1 << S5P_CLKSRC2_G2D_SHIFT);
+#endif
+	__raw_writel(reg, S5P_CLK_SRC2);
+
+	do {
+		reg = __raw_readl(S5P_CLK_MUX_STAT1);
+	} while (reg & ((1 << 7) | (1 << 3)));
+
+	/* 3. DMC1 refresh count for 133MHz if (index == L4) is true
+	 * refresh counter is already programmed in the outer/upper
+	 * code. 0x287@83MHz
+	 **/
+	if (!bus_speed_changing)
+		__raw_writel(0x40d, S5P_VA_DMC1 + 0x30);
+
+	/* 4. SCLKAPLL -> SCLKMPLL */
+	reg = __raw_readl(S5P_CLK_SRC0);
+	reg &= ~(S5P_CLKSRC0_MUX200_MASK);
+	reg |= (0x1 << S5P_CLKSRC0_MUX200_SHIFT);
+	__raw_writel(reg, S5P_CLK_SRC0);
+
+	do {
+		reg = __raw_readl(S5P_CLK_MUX_STAT0);
+	} while (reg & (0x1 << 18));
+
+#if defined(CONFIG_S5PC110_H_TYPE)
+	/* DMC0 source clock: SCLKA2M -> SCLKMPLL */
+	__raw_writel(0x50e, S5P_VA_DMC0 + 0x30);
+
+	reg = __raw_readl(S5P_CLK_DIV6);
+	reg &= ~(S5P_CLKDIV6_ONEDRAM_MASK);
+	reg |= (0x3 << S5P_CLKDIV6_ONEDRAM_SHIFT);
+	__raw_writel(reg, S5P_CLK_DIV6);
+
+	do {
+		reg = __raw_readl(S5P_CLK_DIV_STAT1);
+	} while (reg & (1 << 15));
+
+	reg = __raw_readl(S5P_CLK_SRC6);
+	reg &= ~(S5P_CLKSRC6_ONEDRAM_MASK);
+	reg |= (0x1 << S5P_CLKSRC6_ONEDRAM_SHIFT);
+	__raw_writel(reg, S5P_CLK_SRC6);
+
+	do {
+		reg = __raw_readl(S5P_CLK_MUX_STAT1);
+	} while (reg & (1 << 11));
+#endif
+}
+
+static void s5pv210_target_MPLL2APLL(unsigned int index,
+		unsigned int bus_speed_changing)
+{
+	unsigned int reg;
+
+	/* 7. Set Lock time = 30us*24MHz = 02cf (EVT1) */
+#ifdef CONFIG_CPU_S5PC110_EVT0_ERRATA
+	/* Lock time = 300us*24Mhz = 7200(0x1c20)*/
+	__raw_writel(0x1c20, S5P_APLL_LOCK);
+#else
+	/* Lock time = 30us*24Mhz = 0x2cf*/
+	__raw_writel(0x2cf, S5P_APLL_LOCK);
+#endif
+
+	/* 8. Turn on APLL
+	 * 8-1. Set PMS values
+	 * 8-3. Wait until the PLL is locked
+	 **/
+	if (index == L0)
+		/* APLL FOUT becomes 1000 Mhz */
+		__raw_writel(PLL45XX_APLL_VAL_1000, S5P_APLL_CON);
+	else
+		/* APLL FOUT becomes 800 Mhz */
+		__raw_writel(PLL45XX_APLL_VAL_800, S5P_APLL_CON);
+
+	do {
+		reg = __raw_readl(S5P_APLL_CON);
+	} while (!(reg & (0x1 << 29)));
+
+	/* 9. Change source clock from SCLKMPLL(667MHz)
+	 * to SCLKA2M(200MHz) in MFC_MUX and G3D_MUX
+	 * (667/4=166)->(200/4=50)MHz
+	 **/
+	reg = __raw_readl(S5P_CLK_SRC2);
+	reg &= ~(S5P_CLKSRC2_G3D_MASK | S5P_CLKSRC2_MFC_MASK);
+	reg |= (0 << S5P_CLKSRC2_G3D_SHIFT) | (0 << S5P_CLKSRC2_MFC_SHIFT);
+#ifndef CONFIG_CPU_S5PC110_EVT0_ERRATA
+	reg &= ~S5P_CLKSRC2_G2D_MASK;
+	reg |= 0x1 << S5P_CLKSRC2_G2D_SHIFT;
+#endif
+	__raw_writel(reg, S5P_CLK_SRC2);
+
+	do {
+		reg = __raw_readl(S5P_CLK_MUX_STAT1);
+	} while (reg & ((1 << 7) | (1 << 3)));
+
+	/* 10. Change divider for MFC and G3D
+	 * (200/4=50)->(200/1=200)MHz
+	 **/
+	reg = __raw_readl(S5P_CLK_DIV2);
+	reg &= ~(S5P_CLKDIV2_G3D_MASK | S5P_CLKDIV2_MFC_MASK);
+	reg |= (clkdiv_val[index][10] << S5P_CLKDIV2_G3D_SHIFT) |
+		(clkdiv_val[index][9] << S5P_CLKDIV2_MFC_SHIFT);
+#ifndef CONFIG_CPU_S5PC110_EVT0_ERRATA
+	reg &= ~S5P_CLKDIV2_G2D_MASK;
+	reg |= 0x2 << S5P_CLKDIV2_G2D_SHIFT;
+#endif
+	__raw_writel(reg, S5P_CLK_DIV2);
+
+	do {
+		reg = __raw_readl(S5P_CLK_DIV_STAT0);
+	} while (reg & ((1 << 16) | (1 << 17)));
+
+	/* 11. Change MPLL to APLL in MSYS_MUX */
+	reg = __raw_readl(S5P_CLK_SRC0);
+	reg &= ~(S5P_CLKSRC0_MUX200_MASK);
+	reg |= (0x0 << S5P_CLKSRC0_MUX200_SHIFT); /* SCLKMPLL -> SCLKAPLL   */
+	__raw_writel(reg, S5P_CLK_SRC0);
+
+	do {
+		reg = __raw_readl(S5P_CLK_MUX_STAT0);
+	} while (reg & (0x1 << 18));
+
+	/* 12. DMC1 refresh counter
+	 * L4: DMC1 = 100MHz 7.8us/(1/100) = 0x30c
+	 * Others: DMC1 = 200MHz 7.8us/(1/200) = 0x618
+	 **/
+	if (!bus_speed_changing)
+		__raw_writel(0x618, S5P_VA_DMC1 + 0x30);
+
+#if defined(CONFIG_S5PC110_H_TYPE)
+	/* DMC0 source clock: SCLKMPLL -> SCLKA2M */
+	reg = __raw_readl(S5P_CLK_SRC6);
+	reg &= ~(S5P_CLKSRC6_ONEDRAM_MASK);
+	reg |= (0x0 << S5P_CLKSRC6_ONEDRAM_SHIFT);
+	__raw_writel(reg, S5P_CLK_SRC6);
+
+	do {
+		reg = __raw_readl(S5P_CLK_MUX_STAT1);
+	} while (reg & (1 << 11));
+
+	reg = __raw_readl(S5P_CLK_DIC6);
+	reg &= ~(S5P_CLKDIV6_ONEDRAM_MASK);
+	reg |= (0x0 << S5P_CLKDIV6_ONEDRAM_SHIFT);
+	__raw_writel(reg, S5P_CLK_DIV6);
+
+	do {
+		reg = __raw_readl(S5P_CLK_DIV_STAT1);
+	} while (reg & (1 << 15));
+	__raw_writel(0x618, S5P_VA_DMC0 + 0x30);
+#endif
+}
+
+#ifdef CONFIG_PM
+static int no_cpufreq_access;
+/* s5pv210_target: relation has an additional symantics other than
+ * the standard
+ * [0x30]:
+ *	1: disable further access to target until being re-enabled.
+ *	2: re-enable access to target */
+#endif
+static int s5pv210_target(struct cpufreq_policy *policy,
+		unsigned int target_freq,
+		unsigned int relation)
+{
+	static int initialized;
+	int ret = 0;
+	unsigned long arm_clk;
+	unsigned int index, reg, arm_volt, int_volt;
+	unsigned int pll_changing = 0;
+	unsigned int bus_speed_changing = 0;
+	int ramp_req_delay, ramp_real_delay;
+	struct timeval start, end;
+
+#ifdef CONFIG_CPU_FREQ_DEBUG
+	printk(KERN_INFO "cpufreq: Entering for %dkHz\n", target_freq);
+#endif
+#ifdef CONFIG_PM
+	if ((relation & ENABLE_FURTHER_CPUFREQ) &&
+			(relation & DISABLE_FURTHER_CPUFREQ)) {
+		/* Invalidate both if both marked */
+		relation &= ~ENABLE_FURTHER_CPUFREQ;
+		relation &= ~DISABLE_FURTHER_CPUFREQ;
+		printk(KERN_ERR "%s:%d denied marking \"FURTHER_CPUFREQ\""
+				" as both marked.\n",
+				__FILE__, __LINE__);
+	}
+	if (relation & ENABLE_FURTHER_CPUFREQ)
+		no_cpufreq_access = 0;
+	if (no_cpufreq_access == 1) {
+#ifdef CONFIG_PM_VERBOSE
+		printk(KERN_ERR "%s:%d denied access to %s as it is disabled"
+			       " temporarily\n", __FILE__, __LINE__, __func__);
+#endif
+		ret = -EINVAL;
+		goto out;
+	}
+	if (relation & DISABLE_FURTHER_CPUFREQ)
+		no_cpufreq_access = 1;
+	relation &= ~MASK_FURTHER_CPUFREQ;
+#endif
+
+	s3c_freqs.freqs.old = s5pv210_getspeed(0);
+
+	if (cpufreq_frequency_table_target(policy, s5pv210_freq_table,
+				target_freq, relation, &index)) {
+		ret = -EINVAL;
+		goto out;
+	}
+#ifdef CPUFREQ_DISABLE_100MHZ
+	if (index == L4)
+		index = L3;
+#endif
+#ifdef CPUFREQ_DISABLE_1GHZ
+	if (index == L0)
+		index = L1;
+#endif
+
+	arm_clk = s5pv210_freq_table[index].frequency;
+
+	s3c_freqs.freqs.new = arm_clk;
+	s3c_freqs.freqs.cpu = 0;
+
+	if (s3c_freqs.freqs.new == s3c_freqs.freqs.old &&
+			initialized >= 2)
+		return 0;
+	else if (initialized < 2)
+		initialized++;
+
+	arm_volt = s5pv210_dvs_conf[index].arm_volt;
+	int_volt = s5pv210_dvs_conf[index].int_volt;
+
+	/* iNew clock information update */
+	memcpy(&s3c_freqs.new, &s5pv210_clk_info[index],
+			sizeof(struct s3c_freq));
+
+	if (s3c_freqs.freqs.new >= s3c_freqs.freqs.old) {
+		/* Voltage up code: increase ARM first */
+		if (!IS_ERR(arm_regulator) && !IS_ERR(internal_regulator)) {
+			regulator_set_voltage(arm_regulator,
+					arm_volt, arm_volt);
+			regulator_set_voltage(internal_regulator,
+					int_volt, int_volt);
+		}
+#if defined(USE_FIXED_RAMP_UP_DELAY)
+		ramp_req_delay = (((arm_volt - previous_arm_volt) / 1000) /
+				PMIC_RAMP_UP);
+		udelay(ramp_req_delay);
+#else
+		do_gettimeofday(&start);
+#endif
+	}
+	cpufreq_notify_transition(&s3c_freqs.freqs, CPUFREQ_PRECHANGE);
+
+	if (s3c_freqs.new.fclk != s3c_freqs.old.fclk || initialized < 2)
+		pll_changing = 1;
+
+	if (s3c_freqs.new.hclk_msys != s3c_freqs.old.hclk_msys ||
+			initialized < 2)
+		bus_speed_changing = 1;
+
+	if (bus_speed_changing) {
+		/* Reconfigure DRAM refresh counter value for minimum
+		 * temporary clock while changing divider.
+		 * expected clock is 83MHz: 7.8usec/(1/83MHz) = 0x287
+		 **/
+		if (pll_changing)
+			__raw_writel(0x287, S5P_VA_DMC1 + 0x30);
+		else
+			__raw_writel(0x30c, S5P_VA_DMC1 + 0x30);
+
+		__raw_writel(0x287, S5P_VA_DMC0 + 0x30);
+	}
+
+	/* APLL should be changed in this level
+	 * APLL -> MPLL(for stable transition) -> APLL
+	 * Some clock source's clock API  are not prepared. Do not use clock API
+	 * in below code.
+	 */
+	if (pll_changing)
+		s5pv210_target_APLL2MPLL(index, bus_speed_changing);
+
+	/* ARM MCS value changed */
+	if (index != L4) {
+		reg = __raw_readl(S5P_ARM_MCS_CON);
+		reg &= ~0x3;
+		reg |= 0x1;
+		__raw_writel(reg, S5P_ARM_MCS_CON);
+	}
+
+#if !defined(USE_FIXED_RAMP_UP_DELAY)
+	if (s3c_freqs.freqs.new > s3c_freqs.freqs.old) {
+		do_gettimeofday(&end);
+
+		/* Based on 10mV/used ramp up speed
+		 * Required ramp up delay time (usec) for this voltage change
+		 **/
+		ramp_req_delay = (((arm_volt - previous_arm_volt) / 1000) /
+				PMIC_RAMP_UP);
+		ramp_real_delay = ramp_req_delay -
+			((end.tv_sec - start.tv_sec) * USEC_PER_SEC +
+			 (end.tv_usec - start.tv_usec));
+
+		if (ramp_real_delay > 0) {
+			udelay(ramp_real_delay);
+		} else {
+#ifdef CONFIG_PM_VERBOSE
+			printk(KERN_INFO "Ramp up delay already consumed"
+					"[%d]\n", ramp_real_delay);
+#endif
+		}
+	}
+#endif
+
+	reg = __raw_readl(S5P_CLK_DIV0);
+
+	reg &= ~(S5P_CLKDIV0_APLL_MASK | S5P_CLKDIV0_A2M_MASK
+			| S5P_CLKDIV0_HCLK200_MASK | S5P_CLKDIV0_PCLK100_MASK
+			| S5P_CLKDIV0_HCLK166_MASK | S5P_CLKDIV0_PCLK83_MASK
+			| S5P_CLKDIV0_HCLK133_MASK | S5P_CLKDIV0_PCLK66_MASK);
+
+	reg |= ((clkdiv_val[index][0]<<S5P_CLKDIV0_APLL_SHIFT)
+			| (clkdiv_val[index][1] << S5P_CLKDIV0_A2M_SHIFT)
+			| (clkdiv_val[index][2] << S5P_CLKDIV0_HCLK200_SHIFT)
+			| (clkdiv_val[index][3] << S5P_CLKDIV0_PCLK100_SHIFT)
+			| (clkdiv_val[index][4] << S5P_CLKDIV0_HCLK166_SHIFT)
+			| (clkdiv_val[index][5] << S5P_CLKDIV0_PCLK83_SHIFT)
+			| (clkdiv_val[index][6] << S5P_CLKDIV0_HCLK133_SHIFT)
+			| (clkdiv_val[index][7] << S5P_CLKDIV0_PCLK66_SHIFT));
+
+	__raw_writel(reg, S5P_CLK_DIV0);
+
+	do {
+		reg = __raw_readl(S5P_CLK_DIV_STAT0);
+	} while (reg & 0xff);
+
+	/* ARM MCS value changed */
+	if (index == L4) {
+		reg = __raw_readl(S5P_ARM_MCS_CON);
+		reg &= ~0x3;
+		reg |= 0x3;
+		__raw_writel(reg, S5P_ARM_MCS_CON);
+	}
+
+	if (pll_changing)
+		s5pv210_target_MPLL2APLL(index, bus_speed_changing);
+
+	/* L4 level need to change memory bus speed,
+	 * hence onedram clock divier and
+	 * memory refresh parameter should be changed
+	 */
+	if (bus_speed_changing) {
+		reg = __raw_readl(S5P_CLK_DIV6);
+		reg &= ~S5P_CLKDIV6_ONEDRAM_MASK;
+		reg |= (clkdiv_val[index][8] << S5P_CLKDIV6_ONEDRAM_SHIFT);
+		/* ONEDRAM Clock Divider Ratio: 7 for L4, 3 for Others */
+		__raw_writel(reg, S5P_CLK_DIV6);
+
+		do {
+			reg = __raw_readl(S5P_CLK_DIV_STAT1);
+		} while (reg & (1 << 15));
+
+		/* Reconfigure DRAM refresh counter value */
+		if (index != L4) {
+			/* DMC0: 166MHz
+			 * DMC1: 200MHz
+			 **/
+			__raw_writel(0x618, S5P_VA_DMC1 + 0x30);
+#if !defined(CONFIG_S5PC110_H_TYPE)
+			__raw_writel(0x50e, S5P_VA_DMC0 + 0x30);
+#else
+			__raw_writel(0x618, S5P_VA_DMC0 + 0x30);
+#endif
+		} else {
+			/* DMC0: 83MHz
+			 * DMC1: 100MHz
+			 **/
+			__raw_writel(0x30c, S5P_VA_DMC1 + 0x30);
+			__raw_writel(0x287, S5P_VA_DMC0 + 0x30);
+		}
+	}
+
+	if (s3c_freqs.freqs.new < s3c_freqs.freqs.old) {
+		/* Voltage down: decrease INT first.*/
+		if (!IS_ERR(arm_regulator) && !IS_ERR(internal_regulator)) {
+			regulator_set_voltage(internal_regulator,
+					int_volt, int_volt);
+			regulator_set_voltage(arm_regulator,
+					arm_volt, arm_volt);
+		}
+	}
+	cpufreq_notify_transition(&s3c_freqs.freqs, CPUFREQ_POSTCHANGE);
+
+	memcpy(&s3c_freqs.old, &s3c_freqs.new, sizeof(struct s3c_freq));
+#ifdef CONFIG_CPU_FREQ_DEBUG
+	printk(KERN_INFO "cpufreq: Performance changed[L%d]\n", index);
+#endif
+	previous_arm_volt = s5pv210_dvs_conf[index].arm_volt;
+out:
+	return ret;
+}
+
+
+#ifdef CONFIG_PM
+static int previous_frequency;
+
+static int s5pv210_cpufreq_suspend(struct cpufreq_policy *policy,
+		pm_message_t pmsg)
+{
+	int ret = 0;
+	printk(KERN_INFO "cpufreq: Entering suspend.\n");
+
+	previous_frequency = cpufreq_get(0);
+	ret = __cpufreq_driver_target(cpufreq_cpu_get(0), SLEEP_FREQ,
+			DISABLE_FURTHER_CPUFREQ);
+	return ret;
+}
+
+static int s5pv210_cpufreq_resume(struct cpufreq_policy *policy)
+{
+	int ret = 0;
+	u32 rate;
+	int level = CPUFREQ_TABLE_END;
+	int i;
+
+	printk(KERN_INFO "cpufreq: Waking up from a suspend.\n");
+
+	__cpufreq_driver_target(cpufreq_cpu_get(0), previous_frequency,
+			ENABLE_FURTHER_CPUFREQ);
+
+	/* Clock information update with wakeup value */
+	rate = clk_get_rate(mpu_clk);
+
+	i = 0;
+	while (s5pv210_freq_table[i].frequency != CPUFREQ_TABLE_END) {
+		if (s5pv210_freq_table[i].frequency * 1000 == rate) {
+			level = s5pv210_freq_table[i].index;
+			break;
+		}
+		i++;
+	}
+
+	if (level == CPUFREQ_TABLE_END) { /* Not found */
+		printk(KERN_ERR "[%s:%d] clock speed does not match: "
+				"%d. Using L1 of 800MHz.\n",
+				__FILE__, __LINE__, rate);
+		level = L1;
+	}
+
+	memcpy(&s3c_freqs.old, &s5pv210_clk_info[level],
+			sizeof(struct s3c_freq));
+	previous_arm_volt = s5pv210_dvs_conf[level].arm_volt;
+	return ret;
+}
+#endif
+
+
+static int __init s5pv210_cpu_init(struct cpufreq_policy *policy)
+{
+	u32 reg, rate, apllreg;
+	int i, level = CPUFREQ_TABLE_END;
+
+	printk(KERN_INFO "S5PV210 CPUFREQ %s:%d\n", __FILE__, __LINE__);
+#ifdef CONFIG_PM
+	no_cpufreq_access = 0;
+#endif
+#ifdef CLK_OUT_PROBING
+	reg = __raw_readl(S5P_CLK_OUT);
+	reg &= ~(0x1f << 12 | 0xf << 20); /* CLKSEL and DIVVAL*/
+	reg |= (0xf << 12 | 0x1 << 20); /* CLKSEL = ARMCLK/4, DIVVAL = 1 */
+	/* Result = ARMCLK / 4 / ( 1 + 1 ) */
+	__raw_writel(reg, S5P_CLK_OUT);
+#endif
+	mpu_clk = clk_get(NULL, MPU_CLK);
+	if (IS_ERR(mpu_clk)) {
+		printk(KERN_ERR "S5PV210 CPUFREQ cannot get MPU_CLK(%s)\n",
+				MPU_CLK);
+		return PTR_ERR(mpu_clk);
+	}
+#if defined(CONFIG_REGULATOR)
+#ifdef CONFIG_REGULATOR_MAX8998
+	reg = __raw_readl(S5P_CLK_OUT);
+	reg &= ~(0x1f << 12 | 0xf << 20); /* CLKSEL and DIVVAL*/
+	apllreg = __raw_readl(S5P_APLL_CON);
+#ifdef CPUFREQ_DISABLE_1GHZ
+	reg |= (0x0 << 12 | 0x9 << 20);	/* (800/4) / (9+1) = 20 */
+#else
+	if (((apllreg << 16) & 0x3ff) == 0xfa) /* APLL=1GHz */
+		reg |= (0x0 << 12 | 0xb << 20);	/* (1000/4) / (11+1) = 20.833 */
+	else /* APLL=800MHz */
+		reg |= (0x0 << 12 | 0x9 << 20);	/* (800/4) / (9+1) = 20 */
+#endif
+	__raw_writel(reg, S5P_CLK_OUT);
+#endif
+#endif
+
+	if (policy->cpu != 0) {
+		printk(KERN_ERR "S5PV210 CPUFREQ cannot get proper cpu(%d)\n",
+				policy->cpu);
+		return -EINVAL;
+	}
+	policy->cur = policy->min = policy->max = s5pv210_getspeed(0);
+
+	cpufreq_frequency_table_get_attr(s5pv210_freq_table, policy->cpu);
+
+	policy->cpuinfo.transition_latency = 40000;	/*1us*/
+
+	rate = clk_get_rate(mpu_clk);
+	i = 0;
+
+	while (s5pv210_freq_table[i].frequency != CPUFREQ_TABLE_END) {
+		if (s5pv210_freq_table[i].frequency * 1000 == rate) {
+			level = s5pv210_freq_table[i].index;
+			break;
+		}
+		i++;
+	}
+
+	if (level == CPUFREQ_TABLE_END) { /* Not found */
+		printk(KERN_ERR "[%s:%d] clock speed does not match: "
+				"%d. Using L1 of 800MHz.\n",
+				__FILE__, __LINE__, rate);
+		level = L1;
+	}
+
+	printk(KERN_INFO "S5PV210 CPUFREQ Initialized.\n");
+
+	memcpy(&s3c_freqs.old, &s5pv210_clk_info[level],
+			sizeof(struct s3c_freq));
+	previous_arm_volt = s5pv210_dvs_conf[level].arm_volt;
+
+	return cpufreq_frequency_table_cpuinfo(policy, s5pv210_freq_table);
+}
+
+static struct cpufreq_driver s5pv210_driver = {
+	.flags		= CPUFREQ_STICKY,
+	.verify		= s5pv210_verify_speed,
+	.target		= s5pv210_target,
+	.get		= s5pv210_getspeed,
+	.init		= s5pv210_cpu_init,
+	.name		= "s5pv210",
+#ifdef CONFIG_PM
+	.suspend	= s5pv210_cpufreq_suspend,
+	.resume		= s5pv210_cpufreq_resume,
+#endif
+};
+
+static int __init s5pv210_cpufreq_init(void)
+{
+	printk(KERN_INFO "S5PV210 CPUFREQ Init.\n");
+#ifdef CONFIG_REGULATOR
+	arm_regulator = regulator_get_exclusive(NULL, "vddarm");
+	if (IS_ERR(arm_regulator)) {
+		printk(KERN_ERR "failed to get regulater resource vddarm\n");
+		goto error;
+	}
+	internal_regulator = regulator_get_exclusive(NULL, "vddint");
+	if (IS_ERR(internal_regulator)) {
+		printk(KERN_ERR "failed to get regulater resource vddint\n");
+		goto error;
+	}
+#endif
+	goto finish;
+error:
+	printk(KERN_WARNING "Cannot get vddarm or vddint. CPUFREQ Will not"
+		       " change the voltage.\n");
+finish:
+	return cpufreq_register_driver(&s5pv210_driver);
+}
+
+late_initcall(s5pv210_cpufreq_init);
diff --git a/arch/arm/mach-s5pv210/include/mach/cpu-freq.h b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
new file mode 100644
index 0000000..d3c31b2
--- /dev/null
+++ b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
@@ -0,0 +1,50 @@
+/* arch/arm/mach-s5pv210/include/mach/cpu-freq.h
+ *
+ * Copyright (c) 2010 Samsung Electronics
+ *
+ * 	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * S5PV210/S5PC110 CPU frequency scaling support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
+#define _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
+
+#include <linux/cpufreq.h>
+
+#ifdef CONFIG_CPU_S5PV210
+
+#define USE_FREQ_TABLE
+
+#define KHZ_T           1000
+
+#define MPU_CLK         "armclk"
+
+enum perf_level {
+	L0 = 0,
+	L1,
+	L2,
+	L3,
+	L4,
+};
+
+#define CLK_DIV0_MASK   ((0x7<<0)|(0x7<<8)|(0x7<<12))   /* APLL,HCLK_MSYS,PCLK_MSYS mask value  */
+
+#ifdef CONFIG_PM
+#define SLEEP_FREQ      (800 * 1000) /* Use 800MHz when entering sleep */
+
+/* additional symantics for "relation" in cpufreq with pm */
+#define DISABLE_FURTHER_CPUFREQ         0x10
+#define ENABLE_FURTHER_CPUFREQ          0x20
+#define MASK_FURTHER_CPUFREQ            0x30
+/* With 0x00(NOCHANGE), it depends on the previous "further" status */
+
+#endif
+
+
+#endif /* CONFIG_CPU_S5PV210 */
+#endif /* _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_ */
-- 
1.6.3.3

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[Mark Brown]

On Thu, Jul 15, 2010 at 06:06:08PM +0900, MyungJoo Ham wrote:

> +/* Based on MAX8998 default RAMP time: 10mV/us */
> +#define PMIC_RAMP_UP		(10)

This should be handled within the regulator API, not by hard coding
numbers into the CPUfreq drivers.  With many regulators there will be no
need at all for a ramp delay, they can ramp effectively instantaneously.
Doing this through the regulator API avoids hard coding details of an
individual regulator in your driver and ensures that all users of this
regulator can handle the ramp rate.

I did have most of a patch to introduce this, I'll go and dust it off
soon.  You should just be able to remove the ramp rate code from the
CPUfreq driver.

> +       if (s3c_freqs.freqs.new >= s3c_freqs.freqs.old) {
> +               /* Voltage up code: increase ARM first */
> +               if (!IS_ERR(arm_regulator) && !IS_ERR(internal_regulator)) {
> +                       regulator_set_voltage(arm_regulator,
> +                                       arm_volt, arm_volt);
> +                       regulator_set_voltage(internal_regulator,
> +                                       int_volt, int_volt);
> +               }

You shouldn't be specifying an exact voltage here - there's no guarantee
that the exact voltage you request will be delivered by the regulator,
and some boards may wish to use constraints to eliminate some voltages
(eg, to restrict the maximum operating point for the CPU).

What you'd normally have here is that the minimum voltage would be the
desired operating point for the CPU and the maximum voltage would be
fixed at the maximum rated supply for the part.  This will allow the
regulator API to select the lowest voltage it is able to deliver.

> +#ifdef CONFIG_REGULATOR
> +       arm_regulator = regulator_get_exclusive(NULL, "vddarm");
> +       if (IS_ERR(arm_regulator)) {
> +               printk(KERN_ERR "failed to get regulater resource vddarm\n");
> +               goto error;
> +       }
> +       internal_regulator = regulator_get_exclusive(NULL, "vddint");
> +       if (IS_ERR(internal_regulator)) {
> +               printk(KERN_ERR "failed to get regulater resource vddint\n");
> +               goto error;
> +       }
> +#endif

Remove these ifdefs, they're not needed - the regulator API will stub
out the regulators.  You should also have code to check that the voltage
ranges for all the operating modes of the CPU can be supported.  This is
particularly important the way you've got the code at the minute
specifying exact voltages.

> +#if defined(CONFIG_REGULATOR)
> +#ifdef CONFIG_REGULATOR_MAX8998
> +	reg = __raw_readl(S5P_CLK_OUT);
> +	reg &= ~(0x1f << 12 | 0xf << 20); /* CLKSEL and DIVVAL*/
> +	apllreg = __raw_readl(S5P_APLL_CON);

This ifdef for a particular regulator looks *very* suspicious.  What's
going on here?

> +#ifdef CPUFREQ_DISABLE_1GHZ
> +	reg |= (0x0 << 12 | 0x9 << 20);	/* (800/4) / (9+1) = 20 */
> +#else
> +	if (((apllreg << 16) & 0x3ff) == 0xfa) /* APLL=1GHz */
> +		reg |= (0x0 << 12 | 0xb << 20);	/* (1000/4) / (11+1) = 20.833 */
> +	else /* APLL=800MHz */
> +		reg |= (0x0 << 12 | 0x9 << 20);	/* (800/4) / (9+1) = 20 */
> +#endif
> +	__raw_writel(reg, S5P_CLK_OUT);
> +#endif
> +#endif
> +
> +	if (policy->cpu != 0) {
> +		printk(KERN_ERR "S5PV210 CPUFREQ cannot get proper cpu(%d)\n",
> +				policy->cpu);
> +		return -EINVAL;
> +	}
> +	policy->cur = policy->min = policy->max = s5pv210_getspeed(0);
> +
> +	cpufreq_frequency_table_get_attr(s5pv210_freq_table, policy->cpu);
> +
> +	policy->cpuinfo.transition_latency = 40000;	/*1us*/
> +
> +	rate = clk_get_rate(mpu_clk);
> +	i = 0;
> +
> +	while (s5pv210_freq_table[i].frequency != CPUFREQ_TABLE_END) {
> +		if (s5pv210_freq_table[i].frequency * 1000 == rate) {
> +			level = s5pv210_freq_table[i].index;
> +			break;
> +		}
> +		i++;
> +	}
> +
> +	if (level == CPUFREQ_TABLE_END) { /* Not found */
> +		printk(KERN_ERR "[%s:%d] clock speed does not match: "
> +				"%d. Using L1 of 800MHz.\n",
> +				__FILE__, __LINE__, rate);
> +		level = L1;
> +	}
> +
> +	printk(KERN_INFO "S5PV210 CPUFREQ Initialized.\n");
> +
> +	memcpy(&s3c_freqs.old, &s5pv210_clk_info[level],
> +			sizeof(struct s3c_freq));
> +	previous_arm_volt = s5pv210_dvs_conf[level].arm_volt;
> +
> +	return cpufreq_frequency_table_cpuinfo(policy, s5pv210_freq_table);
> +}
> +
> +static struct cpufreq_driver s5pv210_driver = {
> +	.flags		= CPUFREQ_STICKY,
> +	.verify		= s5pv210_verify_speed,
> +	.target		= s5pv210_target,
> +	.get		= s5pv210_getspeed,
> +	.init		= s5pv210_cpu_init,
> +	.name		= "s5pv210",
> +#ifdef CONFIG_PM
> +	.suspend	= s5pv210_cpufreq_suspend,
> +	.resume		= s5pv210_cpufreq_resume,
> +#endif
> +};
> +
> +static int __init s5pv210_cpufreq_init(void)
> +{
> +	printk(KERN_INFO "S5PV210 CPUFREQ Init.\n");
> +#ifdef CONFIG_REGULATOR
> +	arm_regulator = regulator_get_exclusive(NULL, "vddarm");
> +	if (IS_ERR(arm_regulator)) {
> +		printk(KERN_ERR "failed to get regulater resource vddarm\n");
> +		goto error;
> +	}
> +	internal_regulator = regulator_get_exclusive(NULL, "vddint");
> +	if (IS_ERR(internal_regulator)) {
> +		printk(KERN_ERR "failed to get regulater resource vddint\n");
> +		goto error;
> +	}
> +#endif
> +	goto finish;
> +error:
> +	printk(KERN_WARNING "Cannot get vddarm or vddint. CPUFREQ Will not"
> +		       " change the voltage.\n");
> +finish:
> +	return cpufreq_register_driver(&s5pv210_driver);
> +}
> +
> +late_initcall(s5pv210_cpufreq_init);
> diff --git a/arch/arm/mach-s5pv210/include/mach/cpu-freq.h b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
> new file mode 100644
> index 0000000..d3c31b2
> --- /dev/null
> +++ b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
> @@ -0,0 +1,50 @@
> +/* arch/arm/mach-s5pv210/include/mach/cpu-freq.h
> + *
> + * Copyright (c) 2010 Samsung Electronics
> + *
> + * 	MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * S5PV210/S5PC110 CPU frequency scaling support
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> +*/
> +
> +#ifndef _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
> +#define _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
> +
> +#include <linux/cpufreq.h>
> +
> +#ifdef CONFIG_CPU_S5PV210
> +
> +#define USE_FREQ_TABLE
> +
> +#define KHZ_T           1000
> +
> +#define MPU_CLK         "armclk"
> +
> +enum perf_level {
> +	L0 = 0,
> +	L1,
> +	L2,
> +	L3,
> +	L4,
> +};
> +
> +#define CLK_DIV0_MASK   ((0x7<<0)|(0x7<<8)|(0x7<<12))   /* APLL,HCLK_MSYS,PCLK_MSYS mask value  */
> +
> +#ifdef CONFIG_PM
> +#define SLEEP_FREQ      (800 * 1000) /* Use 800MHz when entering sleep */
> +
> +/* additional symantics for "relation" in cpufreq with pm */
> +#define DISABLE_FURTHER_CPUFREQ         0x10
> +#define ENABLE_FURTHER_CPUFREQ          0x20
> +#define MASK_FURTHER_CPUFREQ            0x30
> +/* With 0x00(NOCHANGE), it depends on the previous "further" status */
> +
> +#endif
> +
> +
> +#endif /* CONFIG_CPU_S5PV210 */
> +#endif /* _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_ */
> -- 
> 1.6.3.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[MyungJoo Ham]

On Thu, Jul 15, 2010 at 8:59 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Jul 15, 2010 at 06:06:08PM +0900, MyungJoo Ham wrote:
>
>> +/* Based on MAX8998 default RAMP time: 10mV/us */
>> +#define PMIC_RAMP_UP ? ? ? ? (10)
>
> This should be handled within the regulator API, not by hard coding
> numbers into the CPUfreq drivers. ?With many regulators there will be no
> need at all for a ramp delay, they can ramp effectively instantaneously.
> Doing this through the regulator API avoids hard coding details of an
> individual regulator in your driver and ensures that all users of this
> regulator can handle the ramp rate.
>
> I did have most of a patch to introduce this, I'll go and dust it off
> soon. ?You should just be able to remove the ramp rate code from the
> CPUfreq driver.

Making regulator API wait for the ramp up seems to be something like
"synchronous I/O", which in turn, forces CPU to do nothing while CPU
may execute some instructions while waiting for the ramp up. However,
we just do udelay() anyway; thus, it doesn't matter for us as well.
I'll try to put those udelay into MAX8998 PMIC drivers, which is for
S5PV210. I'll remove these udelay and PMIC_RAMP)UP from cpufreq driver
with the next version.

>
>> + ? ? ? if (s3c_freqs.freqs.new >= s3c_freqs.freqs.old) {
>> + ? ? ? ? ? ? ? /* Voltage up code: increase ARM first */
>> + ? ? ? ? ? ? ? if (!IS_ERR(arm_regulator) && !IS_ERR(internal_regulator)) {
>> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(arm_regulator,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? arm_volt, arm_volt);
>> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(internal_regulator,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int_volt, int_volt);
>> + ? ? ? ? ? ? ? }
>
> You shouldn't be specifying an exact voltage here - there's no guarantee
> that the exact voltage you request will be delivered by the regulator,
> and some boards may wish to use constraints to eliminate some voltages
> (eg, to restrict the maximum operating point for the CPU).
>
> What you'd normally have here is that the minimum voltage would be the
> desired operating point for the CPU and the maximum voltage would be
> fixed at the maximum rated supply for the part. ?This will allow the
> regulator API to select the lowest voltage it is able to deliver.

Currently, there is a bug in MAX8998 PMIC driver that sets VDDARM to
minimum possible voltage that is LARGER THAN or EQUAL TO max_vol, not
min_vol. I'll correct set_voltage usage of CPUFREQ assuming that
bugfix for MAX8998 PMIC driver is sent. (Joonyoung Shim, who's just
started submitting MAX8998 PMIC driver, know about this issue and will
probably send a patch soon.)

Modifying set_voltage usage of CPUFREQ as you've told will also make
the driver compatible to AVS; I'm working on AVS - IEM as well. :)

>
>> +#ifdef CONFIG_REGULATOR
>> + ? ? ? arm_regulator = regulator_get_exclusive(NULL, "vddarm");
>> + ? ? ? if (IS_ERR(arm_regulator)) {
>> + ? ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddarm\n");
>> + ? ? ? ? ? ? ? goto error;
>> + ? ? ? }
>> + ? ? ? internal_regulator = regulator_get_exclusive(NULL, "vddint");
>> + ? ? ? if (IS_ERR(internal_regulator)) {
>> + ? ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddint\n");
>> + ? ? ? ? ? ? ? goto error;
>> + ? ? ? }
>> +#endif
>
> Remove these ifdefs, they're not needed - the regulator API will stub
> out the regulators. ?You should also have code to check that the voltage
> ranges for all the operating modes of the CPU can be supported. ?This is
> particularly important the way you've got the code at the minute
> specifying exact voltages.

Got it.

>
>> +#if defined(CONFIG_REGULATOR)
>> +#ifdef CONFIG_REGULATOR_MAX8998
>> + ? ? reg = __raw_readl(S5P_CLK_OUT);
>> + ? ? reg &= ~(0x1f << 12 | 0xf << 20); /* CLKSEL and DIVVAL*/
>> + ? ? apllreg = __raw_readl(S5P_APLL_CON);
>
> This ifdef for a particular regulator looks *very* suspicious. ?What's
> going on here?

Good point. That ifdefs should be removed.

Besides, the whole section enclosed by this CONFIG_REGULATOR will be removed.

A while ago, WM8994 had been using CLK_OUT as the clock source with
APLL dependent setting; thus "target" function accessed CLK_OUT when
APLL changed. However, it no longer uses APLL dependent clock source
and we no longer access CLK_OUT at the "target" function; thus, this
code in "init" function can be removed.

>
>> +#ifdef CPUFREQ_DISABLE_1GHZ
>> + ? ? reg |= (0x0 << 12 | 0x9 << 20); /* (800/4) / (9+1) = 20 */
>> +#else
>> + ? ? if (((apllreg << 16) & 0x3ff) == 0xfa) /* APLL=1GHz */
>> + ? ? ? ? ? ? reg |= (0x0 << 12 | 0xb << 20); /* (1000/4) / (11+1) = 20.833 */
>> + ? ? else /* APLL=800MHz */
>> + ? ? ? ? ? ? reg |= (0x0 << 12 | 0x9 << 20); /* (800/4) / (9+1) = 20 */
>> +#endif
>> + ? ? __raw_writel(reg, S5P_CLK_OUT);
>> +#endif
>> +#endif
>> +
>> + ? ? if (policy->cpu != 0) {
>> + ? ? ? ? ? ? printk(KERN_ERR "S5PV210 CPUFREQ cannot get proper cpu(%d)\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? policy->cpu);
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? }
>> + ? ? policy->cur = policy->min = policy->max = s5pv210_getspeed(0);
>> +
>> + ? ? cpufreq_frequency_table_get_attr(s5pv210_freq_table, policy->cpu);
>> +
>> + ? ? policy->cpuinfo.transition_latency = 40000; ? ? /*1us*/
>> +
>> + ? ? rate = clk_get_rate(mpu_clk);
>> + ? ? i = 0;
>> +
>> + ? ? while (s5pv210_freq_table[i].frequency != CPUFREQ_TABLE_END) {
>> + ? ? ? ? ? ? if (s5pv210_freq_table[i].frequency * 1000 == rate) {
>> + ? ? ? ? ? ? ? ? ? ? level = s5pv210_freq_table[i].index;
>> + ? ? ? ? ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? i++;
>> + ? ? }
>> +
>> + ? ? if (level == CPUFREQ_TABLE_END) { /* Not found */
>> + ? ? ? ? ? ? printk(KERN_ERR "[%s:%d] clock speed does not match: "
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%d. Using L1 of 800MHz.\n",
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? __FILE__, __LINE__, rate);
>> + ? ? ? ? ? ? level = L1;
>> + ? ? }
>> +
>> + ? ? printk(KERN_INFO "S5PV210 CPUFREQ Initialized.\n");
>> +
>> + ? ? memcpy(&s3c_freqs.old, &s5pv210_clk_info[level],
>> + ? ? ? ? ? ? ? ? ? ? sizeof(struct s3c_freq));
>> + ? ? previous_arm_volt = s5pv210_dvs_conf[level].arm_volt;
>> +
>> + ? ? return cpufreq_frequency_table_cpuinfo(policy, s5pv210_freq_table);
>> +}
>> +
>> +static struct cpufreq_driver s5pv210_driver = {
>> + ? ? .flags ? ? ? ? ?= CPUFREQ_STICKY,
>> + ? ? .verify ? ? ? ? = s5pv210_verify_speed,
>> + ? ? .target ? ? ? ? = s5pv210_target,
>> + ? ? .get ? ? ? ? ? ?= s5pv210_getspeed,
>> + ? ? .init ? ? ? ? ? = s5pv210_cpu_init,
>> + ? ? .name ? ? ? ? ? = "s5pv210",
>> +#ifdef CONFIG_PM
>> + ? ? .suspend ? ? ? ?= s5pv210_cpufreq_suspend,
>> + ? ? .resume ? ? ? ? = s5pv210_cpufreq_resume,
>> +#endif
>> +};
>> +
>> +static int __init s5pv210_cpufreq_init(void)
>> +{
>> + ? ? printk(KERN_INFO "S5PV210 CPUFREQ Init.\n");
>> +#ifdef CONFIG_REGULATOR
>> + ? ? arm_regulator = regulator_get_exclusive(NULL, "vddarm");
>> + ? ? if (IS_ERR(arm_regulator)) {
>> + ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddarm\n");
>> + ? ? ? ? ? ? goto error;
>> + ? ? }
>> + ? ? internal_regulator = regulator_get_exclusive(NULL, "vddint");
>> + ? ? if (IS_ERR(internal_regulator)) {
>> + ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddint\n");
>> + ? ? ? ? ? ? goto error;
>> + ? ? }
>> +#endif
>> + ? ? goto finish;
>> +error:
>> + ? ? printk(KERN_WARNING "Cannot get vddarm or vddint. CPUFREQ Will not"
>> + ? ? ? ? ? ? ? ? ? ?" change the voltage.\n");
>> +finish:
>> + ? ? return cpufreq_register_driver(&s5pv210_driver);
>> +}
>> +
>> +late_initcall(s5pv210_cpufreq_init);
>> diff --git a/arch/arm/mach-s5pv210/include/mach/cpu-freq.h b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>> new file mode 100644
>> index 0000000..d3c31b2
>> --- /dev/null
>> +++ b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>> @@ -0,0 +1,50 @@
>> +/* arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>> + *
>> + * Copyright (c) 2010 Samsung Electronics
>> + *
>> + * ? MyungJoo Ham <myungjoo.ham@samsung.com>
>> + *
>> + * S5PV210/S5PC110 CPU frequency scaling support
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> +*/
>> +
>> +#ifndef _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
>> +#define _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
>> +
>> +#include <linux/cpufreq.h>
>> +
>> +#ifdef CONFIG_CPU_S5PV210
>> +
>> +#define USE_FREQ_TABLE
>> +
>> +#define KHZ_T ? ? ? ? ? 1000
>> +
>> +#define MPU_CLK ? ? ? ? "armclk"
>> +
>> +enum perf_level {
>> + ? ? L0 = 0,
>> + ? ? L1,
>> + ? ? L2,
>> + ? ? L3,
>> + ? ? L4,
>> +};
>> +
>> +#define CLK_DIV0_MASK ? ((0x7<<0)|(0x7<<8)|(0x7<<12)) ? /* APLL,HCLK_MSYS,PCLK_MSYS mask value ?*/
>> +
>> +#ifdef CONFIG_PM
>> +#define SLEEP_FREQ ? ? ?(800 * 1000) /* Use 800MHz when entering sleep */
>> +
>> +/* additional symantics for "relation" in cpufreq with pm */
>> +#define DISABLE_FURTHER_CPUFREQ ? ? ? ? 0x10
>> +#define ENABLE_FURTHER_CPUFREQ ? ? ? ? ?0x20
>> +#define MASK_FURTHER_CPUFREQ ? ? ? ? ? ?0x30
>> +/* With 0x00(NOCHANGE), it depends on the previous "further" status */
>> +
>> +#endif
>> +
>> +
>> +#endif /* CONFIG_CPU_S5PV210 */
>> +#endif /* _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_ */
>> --
>> 1.6.3.3
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
> --
> "You grabbed my hand and we fell into it, like a daydream - or a fever."
>

Thanks!

-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[Kyungmin Park]

On Fri, Jul 16, 2010 at 2:37 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> On Thu, Jul 15, 2010 at 8:59 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> On Thu, Jul 15, 2010 at 06:06:08PM +0900, MyungJoo Ham wrote:
>>
>>> +/* Based on MAX8998 default RAMP time: 10mV/us */
>>> +#define PMIC_RAMP_UP ? ? ? ? (10)
>>
>> This should be handled within the regulator API, not by hard coding
>> numbers into the CPUfreq drivers. ?With many regulators there will be no
>> need at all for a ramp delay, they can ramp effectively instantaneously.
>> Doing this through the regulator API avoids hard coding details of an
>> individual regulator in your driver and ensures that all users of this
>> regulator can handle the ramp rate.
>>
>> I did have most of a patch to introduce this, I'll go and dust it off
>> soon. ?You should just be able to remove the ramp rate code from the
>> CPUfreq driver.
>
> Making regulator API wait for the ramp up seems to be something like
> "synchronous I/O", which in turn, forces CPU to do nothing while CPU
> may execute some instructions while waiting for the ramp up. However,
> we just do udelay() anyway; thus, it doesn't matter for us as well.
> I'll try to put those udelay into MAX8998 PMIC drivers, which is for
> S5PV210. I'll remove these udelay and PMIC_RAMP)UP from cpufreq driver
> with the next version.
>
>>
>>> + ? ? ? if (s3c_freqs.freqs.new >= s3c_freqs.freqs.old) {
>>> + ? ? ? ? ? ? ? /* Voltage up code: increase ARM first */
>>> + ? ? ? ? ? ? ? if (!IS_ERR(arm_regulator) && !IS_ERR(internal_regulator)) {
>>> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(arm_regulator,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? arm_volt, arm_volt);
>>> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(internal_regulator,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int_volt, int_volt);
>>> + ? ? ? ? ? ? ? }
>>
>> You shouldn't be specifying an exact voltage here - there's no guarantee
>> that the exact voltage you request will be delivered by the regulator,
>> and some boards may wish to use constraints to eliminate some voltages
>> (eg, to restrict the maximum operating point for the CPU).
>>
>> What you'd normally have here is that the minimum voltage would be the
>> desired operating point for the CPU and the maximum voltage would be
>> fixed at the maximum rated supply for the part. ?This will allow the
>> regulator API to select the lowest voltage it is able to deliver.
>
> Currently, there is a bug in MAX8998 PMIC driver that sets VDDARM to
> minimum possible voltage that is LARGER THAN or EQUAL TO max_vol, not
> min_vol. I'll correct set_voltage usage of CPUFREQ assuming that
> bugfix for MAX8998 PMIC driver is sent. (Joonyoung Shim, who's just
> started submitting MAX8998 PMIC driver, know about this issue and will
> probably send a patch soon.)

Marek submitted the initial max8998 codes. and joonyoung implement the
max8998-rtc.

just explain the bug to marek.

Thank you,
Kyungmin Park

>
> Modifying set_voltage usage of CPUFREQ as you've told will also make
> the driver compatible to AVS; I'm working on AVS - IEM as well. :)
>
>>
>>> +#ifdef CONFIG_REGULATOR
>>> + ? ? ? arm_regulator = regulator_get_exclusive(NULL, "vddarm");
>>> + ? ? ? if (IS_ERR(arm_regulator)) {
>>> + ? ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddarm\n");
>>> + ? ? ? ? ? ? ? goto error;
>>> + ? ? ? }
>>> + ? ? ? internal_regulator = regulator_get_exclusive(NULL, "vddint");
>>> + ? ? ? if (IS_ERR(internal_regulator)) {
>>> + ? ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddint\n");
>>> + ? ? ? ? ? ? ? goto error;
>>> + ? ? ? }
>>> +#endif
>>
>> Remove these ifdefs, they're not needed - the regulator API will stub
>> out the regulators. ?You should also have code to check that the voltage
>> ranges for all the operating modes of the CPU can be supported. ?This is
>> particularly important the way you've got the code at the minute
>> specifying exact voltages.
>
> Got it.
>
>>
>>> +#if defined(CONFIG_REGULATOR)
>>> +#ifdef CONFIG_REGULATOR_MAX8998
>>> + ? ? reg = __raw_readl(S5P_CLK_OUT);
>>> + ? ? reg &= ~(0x1f << 12 | 0xf << 20); /* CLKSEL and DIVVAL*/
>>> + ? ? apllreg = __raw_readl(S5P_APLL_CON);
>>
>> This ifdef for a particular regulator looks *very* suspicious. ?What's
>> going on here?
>
> Good point. That ifdefs should be removed.
>
> Besides, the whole section enclosed by this CONFIG_REGULATOR will be removed.
>
> A while ago, WM8994 had been using CLK_OUT as the clock source with
> APLL dependent setting; thus "target" function accessed CLK_OUT when
> APLL changed. However, it no longer uses APLL dependent clock source
> and we no longer access CLK_OUT at the "target" function; thus, this
> code in "init" function can be removed.
>
>>
>>> +#ifdef CPUFREQ_DISABLE_1GHZ
>>> + ? ? reg |= (0x0 << 12 | 0x9 << 20); /* (800/4) / (9+1) = 20 */
>>> +#else
>>> + ? ? if (((apllreg << 16) & 0x3ff) == 0xfa) /* APLL=1GHz */
>>> + ? ? ? ? ? ? reg |= (0x0 << 12 | 0xb << 20); /* (1000/4) / (11+1) = 20.833 */
>>> + ? ? else /* APLL=800MHz */
>>> + ? ? ? ? ? ? reg |= (0x0 << 12 | 0x9 << 20); /* (800/4) / (9+1) = 20 */
>>> +#endif
>>> + ? ? __raw_writel(reg, S5P_CLK_OUT);
>>> +#endif
>>> +#endif
>>> +
>>> + ? ? if (policy->cpu != 0) {
>>> + ? ? ? ? ? ? printk(KERN_ERR "S5PV210 CPUFREQ cannot get proper cpu(%d)\n",
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? policy->cpu);
>>> + ? ? ? ? ? ? return -EINVAL;
>>> + ? ? }
>>> + ? ? policy->cur = policy->min = policy->max = s5pv210_getspeed(0);
>>> +
>>> + ? ? cpufreq_frequency_table_get_attr(s5pv210_freq_table, policy->cpu);
>>> +
>>> + ? ? policy->cpuinfo.transition_latency = 40000; ? ? /*1us*/
>>> +
>>> + ? ? rate = clk_get_rate(mpu_clk);
>>> + ? ? i = 0;
>>> +
>>> + ? ? while (s5pv210_freq_table[i].frequency != CPUFREQ_TABLE_END) {
>>> + ? ? ? ? ? ? if (s5pv210_freq_table[i].frequency * 1000 == rate) {
>>> + ? ? ? ? ? ? ? ? ? ? level = s5pv210_freq_table[i].index;
>>> + ? ? ? ? ? ? ? ? ? ? break;
>>> + ? ? ? ? ? ? }
>>> + ? ? ? ? ? ? i++;
>>> + ? ? }
>>> +
>>> + ? ? if (level == CPUFREQ_TABLE_END) { /* Not found */
>>> + ? ? ? ? ? ? printk(KERN_ERR "[%s:%d] clock speed does not match: "
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%d. Using L1 of 800MHz.\n",
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? __FILE__, __LINE__, rate);
>>> + ? ? ? ? ? ? level = L1;
>>> + ? ? }
>>> +
>>> + ? ? printk(KERN_INFO "S5PV210 CPUFREQ Initialized.\n");
>>> +
>>> + ? ? memcpy(&s3c_freqs.old, &s5pv210_clk_info[level],
>>> + ? ? ? ? ? ? ? ? ? ? sizeof(struct s3c_freq));
>>> + ? ? previous_arm_volt = s5pv210_dvs_conf[level].arm_volt;
>>> +
>>> + ? ? return cpufreq_frequency_table_cpuinfo(policy, s5pv210_freq_table);
>>> +}
>>> +
>>> +static struct cpufreq_driver s5pv210_driver = {
>>> + ? ? .flags ? ? ? ? ?= CPUFREQ_STICKY,
>>> + ? ? .verify ? ? ? ? = s5pv210_verify_speed,
>>> + ? ? .target ? ? ? ? = s5pv210_target,
>>> + ? ? .get ? ? ? ? ? ?= s5pv210_getspeed,
>>> + ? ? .init ? ? ? ? ? = s5pv210_cpu_init,
>>> + ? ? .name ? ? ? ? ? = "s5pv210",
>>> +#ifdef CONFIG_PM
>>> + ? ? .suspend ? ? ? ?= s5pv210_cpufreq_suspend,
>>> + ? ? .resume ? ? ? ? = s5pv210_cpufreq_resume,
>>> +#endif
>>> +};
>>> +
>>> +static int __init s5pv210_cpufreq_init(void)
>>> +{
>>> + ? ? printk(KERN_INFO "S5PV210 CPUFREQ Init.\n");
>>> +#ifdef CONFIG_REGULATOR
>>> + ? ? arm_regulator = regulator_get_exclusive(NULL, "vddarm");
>>> + ? ? if (IS_ERR(arm_regulator)) {
>>> + ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddarm\n");
>>> + ? ? ? ? ? ? goto error;
>>> + ? ? }
>>> + ? ? internal_regulator = regulator_get_exclusive(NULL, "vddint");
>>> + ? ? if (IS_ERR(internal_regulator)) {
>>> + ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddint\n");
>>> + ? ? ? ? ? ? goto error;
>>> + ? ? }
>>> +#endif
>>> + ? ? goto finish;
>>> +error:
>>> + ? ? printk(KERN_WARNING "Cannot get vddarm or vddint. CPUFREQ Will not"
>>> + ? ? ? ? ? ? ? ? ? ?" change the voltage.\n");
>>> +finish:
>>> + ? ? return cpufreq_register_driver(&s5pv210_driver);
>>> +}
>>> +
>>> +late_initcall(s5pv210_cpufreq_init);
>>> diff --git a/arch/arm/mach-s5pv210/include/mach/cpu-freq.h b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>>> new file mode 100644
>>> index 0000000..d3c31b2
>>> --- /dev/null
>>> +++ b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>>> @@ -0,0 +1,50 @@
>>> +/* arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>>> + *
>>> + * Copyright (c) 2010 Samsung Electronics
>>> + *
>>> + * ? MyungJoo Ham <myungjoo.ham@samsung.com>
>>> + *
>>> + * S5PV210/S5PC110 CPU frequency scaling support
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> +*/
>>> +
>>> +#ifndef _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
>>> +#define _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
>>> +
>>> +#include <linux/cpufreq.h>
>>> +
>>> +#ifdef CONFIG_CPU_S5PV210
>>> +
>>> +#define USE_FREQ_TABLE
>>> +
>>> +#define KHZ_T ? ? ? ? ? 1000
>>> +
>>> +#define MPU_CLK ? ? ? ? "armclk"
>>> +
>>> +enum perf_level {
>>> + ? ? L0 = 0,
>>> + ? ? L1,
>>> + ? ? L2,
>>> + ? ? L3,
>>> + ? ? L4,
>>> +};
>>> +
>>> +#define CLK_DIV0_MASK ? ((0x7<<0)|(0x7<<8)|(0x7<<12)) ? /* APLL,HCLK_MSYS,PCLK_MSYS mask value ?*/
>>> +
>>> +#ifdef CONFIG_PM
>>> +#define SLEEP_FREQ ? ? ?(800 * 1000) /* Use 800MHz when entering sleep */
>>> +
>>> +/* additional symantics for "relation" in cpufreq with pm */
>>> +#define DISABLE_FURTHER_CPUFREQ ? ? ? ? 0x10
>>> +#define ENABLE_FURTHER_CPUFREQ ? ? ? ? ?0x20
>>> +#define MASK_FURTHER_CPUFREQ ? ? ? ? ? ?0x30
>>> +/* With 0x00(NOCHANGE), it depends on the previous "further" status */
>>> +
>>> +#endif
>>> +
>>> +
>>> +#endif /* CONFIG_CPU_S5PV210 */
>>> +#endif /* _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_ */
>>> --
>>> 1.6.3.3
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>
>> --
>> "You grabbed my hand and we fell into it, like a daydream - or a fever."
>>
>
> Thanks!
>
> --
> MyungJoo Ham (???), Ph.D.
> Mobile Software Platform Lab,
> Digital Media and Communications (DMC) Business
> Samsung Electronics
> cell: 82-10-6714-2858
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[MyungJoo Ham]

Dear Marek,

On Fri, Jul 16, 2010 at 2:42 PM, Kyungmin Park
<kyungmin.park@samsung.com> wrote:
> On Fri, Jul 16, 2010 at 2:37 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> On Thu, Jul 15, 2010 at 8:59 PM, Mark Brown
>> <broonie@opensource.wolfsonmicro.com> wrote:
>>> On Thu, Jul 15, 2010 at 06:06:08PM +0900, MyungJoo Ham wrote:
>>>
>>>> +/* Based on MAX8998 default RAMP time: 10mV/us */
>>>> +#define PMIC_RAMP_UP ? ? ? ? (10)
>>>
>>> This should be handled within the regulator API, not by hard coding
>>> numbers into the CPUfreq drivers. ?With many regulators there will be no
>>> need at all for a ramp delay, they can ramp effectively instantaneously.
>>> Doing this through the regulator API avoids hard coding details of an
>>> individual regulator in your driver and ensures that all users of this
>>> regulator can handle the ramp rate.
>>>
>>> I did have most of a patch to introduce this, I'll go and dust it off
>>> soon. ?You should just be able to remove the ramp rate code from the
>>> CPUfreq driver.
>>
>> Making regulator API wait for the ramp up seems to be something like
>> "synchronous I/O", which in turn, forces CPU to do nothing while CPU
>> may execute some instructions while waiting for the ramp up. However,
>> we just do udelay() anyway; thus, it doesn't matter for us as well.
>> I'll try to put those udelay into MAX8998 PMIC drivers, which is for
>> S5PV210. I'll remove these udelay and PMIC_RAMP)UP from cpufreq driver
>> with the next version.
>>
>>>
>>>> + ? ? ? if (s3c_freqs.freqs.new >= s3c_freqs.freqs.old) {
>>>> + ? ? ? ? ? ? ? /* Voltage up code: increase ARM first */
>>>> + ? ? ? ? ? ? ? if (!IS_ERR(arm_regulator) && !IS_ERR(internal_regulator)) {
>>>> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(arm_regulator,
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? arm_volt, arm_volt);
>>>> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(internal_regulator,
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int_volt, int_volt);
>>>> + ? ? ? ? ? ? ? }
>>>
>>> You shouldn't be specifying an exact voltage here - there's no guarantee
>>> that the exact voltage you request will be delivered by the regulator,
>>> and some boards may wish to use constraints to eliminate some voltages
>>> (eg, to restrict the maximum operating point for the CPU).
>>>
>>> What you'd normally have here is that the minimum voltage would be the
>>> desired operating point for the CPU and the maximum voltage would be
>>> fixed at the maximum rated supply for the part. ?This will allow the
>>> regulator API to select the lowest voltage it is able to deliver.
>>
>> Currently, there is a bug in MAX8998 PMIC driver that sets VDDARM to
>> minimum possible voltage that is LARGER THAN or EQUAL TO max_vol, not
>> min_vol. I'll correct set_voltage usage of CPUFREQ assuming that
>> bugfix for MAX8998 PMIC driver is sent. (Joonyoung Shim, who's just
>> started submitting MAX8998 PMIC driver, know about this issue and will
>> probably send a patch soon.)
>
> Marek submitted the initial max8998 codes. and joonyoung implement the
> max8998-rtc.
>
> just explain the bug to marek.
>
> Thank you,
> Kyungmin Park

As mentioned in this thread, we need some corrections in the MAX8998
regulator driver.

1. max8998_set_voltage(rdev, min_uV, max_uV) max/min value

  In the driver, the voltage value set by this function should be the
minimum possible value that is larger than min_uV. However, currently,
it sets the minimum possible value that is larger than or equal to
max_uV, not min_uV. Please correct this one.

2. max8998_set_voltage(rdev, min_uV, max_uV) ramp up delay

 Depending on which rdev we use and RAMP_UP register (including the EN
bit) set_voltage, max8998_set_voltage can execute udelay()
accordingly.


Do you plan to keep updating MAX8998 driver from the old kernel
versions (such as 2.6.32 or 29)? We still have many missing features
in MAX8998 driver with mainline.


>
>>
>> Modifying set_voltage usage of CPUFREQ as you've told will also make
>> the driver compatible to AVS; I'm working on AVS - IEM as well. :)
>>
>>>
>>>> +#ifdef CONFIG_REGULATOR
>>>> + ? ? ? arm_regulator = regulator_get_exclusive(NULL, "vddarm");
>>>> + ? ? ? if (IS_ERR(arm_regulator)) {
>>>> + ? ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddarm\n");
>>>> + ? ? ? ? ? ? ? goto error;
>>>> + ? ? ? }
>>>> + ? ? ? internal_regulator = regulator_get_exclusive(NULL, "vddint");
>>>> + ? ? ? if (IS_ERR(internal_regulator)) {
>>>> + ? ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddint\n");
>>>> + ? ? ? ? ? ? ? goto error;
>>>> + ? ? ? }
>>>> +#endif
>>>
>>> Remove these ifdefs, they're not needed - the regulator API will stub
>>> out the regulators. ?You should also have code to check that the voltage
>>> ranges for all the operating modes of the CPU can be supported. ?This is
>>> particularly important the way you've got the code at the minute
>>> specifying exact voltages.
>>
>> Got it.
>>
>>>
>>>> +#if defined(CONFIG_REGULATOR)
>>>> +#ifdef CONFIG_REGULATOR_MAX8998
>>>> + ? ? reg = __raw_readl(S5P_CLK_OUT);
>>>> + ? ? reg &= ~(0x1f << 12 | 0xf << 20); /* CLKSEL and DIVVAL*/
>>>> + ? ? apllreg = __raw_readl(S5P_APLL_CON);
>>>
>>> This ifdef for a particular regulator looks *very* suspicious. ?What's
>>> going on here?
>>
>> Good point. That ifdefs should be removed.
>>
>> Besides, the whole section enclosed by this CONFIG_REGULATOR will be removed.
>>
>> A while ago, WM8994 had been using CLK_OUT as the clock source with
>> APLL dependent setting; thus "target" function accessed CLK_OUT when
>> APLL changed. However, it no longer uses APLL dependent clock source
>> and we no longer access CLK_OUT at the "target" function; thus, this
>> code in "init" function can be removed.
>>
>>>
>>>> +#ifdef CPUFREQ_DISABLE_1GHZ
>>>> + ? ? reg |= (0x0 << 12 | 0x9 << 20); /* (800/4) / (9+1) = 20 */
>>>> +#else
>>>> + ? ? if (((apllreg << 16) & 0x3ff) == 0xfa) /* APLL=1GHz */
>>>> + ? ? ? ? ? ? reg |= (0x0 << 12 | 0xb << 20); /* (1000/4) / (11+1) = 20.833 */
>>>> + ? ? else /* APLL=800MHz */
>>>> + ? ? ? ? ? ? reg |= (0x0 << 12 | 0x9 << 20); /* (800/4) / (9+1) = 20 */
>>>> +#endif
>>>> + ? ? __raw_writel(reg, S5P_CLK_OUT);
>>>> +#endif
>>>> +#endif
>>>> +
>>>> + ? ? if (policy->cpu != 0) {
>>>> + ? ? ? ? ? ? printk(KERN_ERR "S5PV210 CPUFREQ cannot get proper cpu(%d)\n",
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? policy->cpu);
>>>> + ? ? ? ? ? ? return -EINVAL;
>>>> + ? ? }
>>>> + ? ? policy->cur = policy->min = policy->max = s5pv210_getspeed(0);
>>>> +
>>>> + ? ? cpufreq_frequency_table_get_attr(s5pv210_freq_table, policy->cpu);
>>>> +
>>>> + ? ? policy->cpuinfo.transition_latency = 40000; ? ? /*1us*/
>>>> +
>>>> + ? ? rate = clk_get_rate(mpu_clk);
>>>> + ? ? i = 0;
>>>> +
>>>> + ? ? while (s5pv210_freq_table[i].frequency != CPUFREQ_TABLE_END) {
>>>> + ? ? ? ? ? ? if (s5pv210_freq_table[i].frequency * 1000 == rate) {
>>>> + ? ? ? ? ? ? ? ? ? ? level = s5pv210_freq_table[i].index;
>>>> + ? ? ? ? ? ? ? ? ? ? break;
>>>> + ? ? ? ? ? ? }
>>>> + ? ? ? ? ? ? i++;
>>>> + ? ? }
>>>> +
>>>> + ? ? if (level == CPUFREQ_TABLE_END) { /* Not found */
>>>> + ? ? ? ? ? ? printk(KERN_ERR "[%s:%d] clock speed does not match: "
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%d. Using L1 of 800MHz.\n",
>>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? __FILE__, __LINE__, rate);
>>>> + ? ? ? ? ? ? level = L1;
>>>> + ? ? }
>>>> +
>>>> + ? ? printk(KERN_INFO "S5PV210 CPUFREQ Initialized.\n");
>>>> +
>>>> + ? ? memcpy(&s3c_freqs.old, &s5pv210_clk_info[level],
>>>> + ? ? ? ? ? ? ? ? ? ? sizeof(struct s3c_freq));
>>>> + ? ? previous_arm_volt = s5pv210_dvs_conf[level].arm_volt;
>>>> +
>>>> + ? ? return cpufreq_frequency_table_cpuinfo(policy, s5pv210_freq_table);
>>>> +}
>>>> +
>>>> +static struct cpufreq_driver s5pv210_driver = {
>>>> + ? ? .flags ? ? ? ? ?= CPUFREQ_STICKY,
>>>> + ? ? .verify ? ? ? ? = s5pv210_verify_speed,
>>>> + ? ? .target ? ? ? ? = s5pv210_target,
>>>> + ? ? .get ? ? ? ? ? ?= s5pv210_getspeed,
>>>> + ? ? .init ? ? ? ? ? = s5pv210_cpu_init,
>>>> + ? ? .name ? ? ? ? ? = "s5pv210",
>>>> +#ifdef CONFIG_PM
>>>> + ? ? .suspend ? ? ? ?= s5pv210_cpufreq_suspend,
>>>> + ? ? .resume ? ? ? ? = s5pv210_cpufreq_resume,
>>>> +#endif
>>>> +};
>>>> +
>>>> +static int __init s5pv210_cpufreq_init(void)
>>>> +{
>>>> + ? ? printk(KERN_INFO "S5PV210 CPUFREQ Init.\n");
>>>> +#ifdef CONFIG_REGULATOR
>>>> + ? ? arm_regulator = regulator_get_exclusive(NULL, "vddarm");
>>>> + ? ? if (IS_ERR(arm_regulator)) {
>>>> + ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddarm\n");
>>>> + ? ? ? ? ? ? goto error;
>>>> + ? ? }
>>>> + ? ? internal_regulator = regulator_get_exclusive(NULL, "vddint");
>>>> + ? ? if (IS_ERR(internal_regulator)) {
>>>> + ? ? ? ? ? ? printk(KERN_ERR "failed to get regulater resource vddint\n");
>>>> + ? ? ? ? ? ? goto error;
>>>> + ? ? }
>>>> +#endif
>>>> + ? ? goto finish;
>>>> +error:
>>>> + ? ? printk(KERN_WARNING "Cannot get vddarm or vddint. CPUFREQ Will not"
>>>> + ? ? ? ? ? ? ? ? ? ?" change the voltage.\n");
>>>> +finish:
>>>> + ? ? return cpufreq_register_driver(&s5pv210_driver);
>>>> +}
>>>> +
>>>> +late_initcall(s5pv210_cpufreq_init);
>>>> diff --git a/arch/arm/mach-s5pv210/include/mach/cpu-freq.h b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>>>> new file mode 100644
>>>> index 0000000..d3c31b2
>>>> --- /dev/null
>>>> +++ b/arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>>>> @@ -0,0 +1,50 @@
>>>> +/* arch/arm/mach-s5pv210/include/mach/cpu-freq.h
>>>> + *
>>>> + * Copyright (c) 2010 Samsung Electronics
>>>> + *
>>>> + * ? MyungJoo Ham <myungjoo.ham@samsung.com>
>>>> + *
>>>> + * S5PV210/S5PC110 CPU frequency scaling support
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> +*/
>>>> +
>>>> +#ifndef _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
>>>> +#define _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_
>>>> +
>>>> +#include <linux/cpufreq.h>
>>>> +
>>>> +#ifdef CONFIG_CPU_S5PV210
>>>> +
>>>> +#define USE_FREQ_TABLE
>>>> +
>>>> +#define KHZ_T ? ? ? ? ? 1000
>>>> +
>>>> +#define MPU_CLK ? ? ? ? "armclk"
>>>> +
>>>> +enum perf_level {
>>>> + ? ? L0 = 0,
>>>> + ? ? L1,
>>>> + ? ? L2,
>>>> + ? ? L3,
>>>> + ? ? L4,
>>>> +};
>>>> +
>>>> +#define CLK_DIV0_MASK ? ((0x7<<0)|(0x7<<8)|(0x7<<12)) ? /* APLL,HCLK_MSYS,PCLK_MSYS mask value ?*/
>>>> +
>>>> +#ifdef CONFIG_PM
>>>> +#define SLEEP_FREQ ? ? ?(800 * 1000) /* Use 800MHz when entering sleep */
>>>> +
>>>> +/* additional symantics for "relation" in cpufreq with pm */
>>>> +#define DISABLE_FURTHER_CPUFREQ ? ? ? ? 0x10
>>>> +#define ENABLE_FURTHER_CPUFREQ ? ? ? ? ?0x20
>>>> +#define MASK_FURTHER_CPUFREQ ? ? ? ? ? ?0x30
>>>> +/* With 0x00(NOCHANGE), it depends on the previous "further" status */
>>>> +
>>>> +#endif
>>>> +
>>>> +
>>>> +#endif /* CONFIG_CPU_S5PV210 */
>>>> +#endif /* _ARCH_ARM_MACH_S5PV210_INCLUDE_MACH_CPU_FREQ_H_ */
>>>> --
>>>> 1.6.3.3
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>>
>>>
>>> --
>>> "You grabbed my hand and we fell into it, like a daydream - or a fever."
>>>
>>
>> Thanks!
>>
>> --
>> MyungJoo Ham (???), Ph.D.
>> Mobile Software Platform Lab,
>> Digital Media and Communications (DMC) Business
>> Samsung Electronics
>> cell: 82-10-6714-2858
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>


Cheers!

- MyungJoo

-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[Mark Brown]

On Fri, Jul 16, 2010 at 02:37:04PM +0900, MyungJoo Ham wrote:
> On Thu, Jul 15, 2010 at 8:59 PM, Mark Brown

> > Doing this through the regulator API avoids hard coding details of an
> > individual regulator in your driver and ensures that all users of this
> > regulator can handle the ramp rate.

> Making regulator API wait for the ramp up seems to be something like
> "synchronous I/O", which in turn, forces CPU to do nothing while CPU
> may execute some instructions while waiting for the ramp up. However,

Yeah, I know, but even if the actual delay is implemented in the
consumer driver the ramp time should still be being queried via the
regulator API so that the details of the specific device aren't hard
coded into the consumer.  In any case as you say...

> we just do udelay() anyway; thus, it doesn't matter for us as well.
> I'll try to put those udelay into MAX8998 PMIC drivers, which is for
> S5PV210. I'll remove these udelay and PMIC_RAMP)UP from cpufreq driver
> with the next version.

...the ramp times are usually vanishingly small so the overwhelming
majority of consumers probably don't care anyway.  If it's a problem we
can add an interface which doesn't do the delay automatically but for
most users it's going to be simpler to just deal with it transparently.

> >> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(arm_regulator,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? arm_volt, arm_volt);

> > You shouldn't be specifying an exact voltage here - there's no guarantee
> > that the exact voltage you request will be delivered by the regulator,
> > and some boards may wish to use constraints to eliminate some voltages
> > (eg, to restrict the maximum operating point for the CPU).

> > What you'd normally have here is that the minimum voltage would be the
> > desired operating point for the CPU and the maximum voltage would be
> > fixed at the maximum rated supply for the part. ?This will allow the
> > regulator API to select the lowest voltage it is able to deliver.

> Currently, there is a bug in MAX8998 PMIC driver that sets VDDARM to
> minimum possible voltage that is LARGER THAN or EQUAL TO max_vol, not
> min_vol. I'll correct set_voltage usage of CPUFREQ assuming that
> bugfix for MAX8998 PMIC driver is sent. (Joonyoung Shim, who's just
> started submitting MAX8998 PMIC driver, know about this issue and will
> probably send a patch soon.)

Oh, fail.  Like you say that's a bug in the regulator driver which will
hopefully be fixed before this gets merged.

> Besides, the whole section enclosed by this CONFIG_REGULATOR will be removed.

> A while ago, WM8994 had been using CLK_OUT as the clock source with
> APLL dependent setting; thus "target" function accessed CLK_OUT when
> APLL changed. However, it no longer uses APLL dependent clock source
> and we no longer access CLK_OUT at the "target" function; thus, this
> code in "init" function can be removed.

Ah, OK.  If other people use that sort of clocking scheme we will need
to come up with a reasonable way of coping with it, but let's save that
for when the problem arises.

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[MyungJoo Ham]

On Fri, Jul 16, 2010 at 5:17 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Jul 16, 2010 at 02:37:04PM +0900, MyungJoo Ham wrote:
>> On Thu, Jul 15, 2010 at 8:59 PM, Mark Brown
>
>> > Doing this through the regulator API avoids hard coding details of an
>> > individual regulator in your driver and ensures that all users of this
>> > regulator can handle the ramp rate.
>
>> Making regulator API wait for the ramp up seems to be something like
>> "synchronous I/O", which in turn, forces CPU to do nothing while CPU
>> may execute some instructions while waiting for the ramp up. However,
>
> Yeah, I know, but even if the actual delay is implemented in the
> consumer driver the ramp time should still be being queried via the
> regulator API so that the details of the specific device aren't hard
> coded into the consumer. ?In any case as you say...
>
>> we just do udelay() anyway; thus, it doesn't matter for us as well.
>> I'll try to put those udelay into MAX8998 PMIC drivers, which is for
>> S5PV210. I'll remove these udelay and PMIC_RAMP)UP from cpufreq driver
>> with the next version.
>
> ...the ramp times are usually vanishingly small so the overwhelming
> majority of consumers probably don't care anyway. ?If it's a problem we
> can add an interface which doesn't do the delay automatically but for
> most users it's going to be simpler to just deal with it transparently.

It's about 30us in this driver (100MHz -> 1GHz), where we lost about
60000 instructions (2 instructions / cycle @ 100MHz, with default RAMP
UP of 10mV/us). However, let's assume that it's ok for now.

>
>> >> + ? ? ? ? ? ? ? ? ? ? ? regulator_set_voltage(arm_regulator,
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? arm_volt, arm_volt);
>
>> > You shouldn't be specifying an exact voltage here - there's no guarantee
>> > that the exact voltage you request will be delivered by the regulator,
>> > and some boards may wish to use constraints to eliminate some voltages
>> > (eg, to restrict the maximum operating point for the CPU).
>
>> > What you'd normally have here is that the minimum voltage would be the
>> > desired operating point for the CPU and the maximum voltage would be
>> > fixed at the maximum rated supply for the part. ?This will allow the
>> > regulator API to select the lowest voltage it is able to deliver.
>
>> Currently, there is a bug in MAX8998 PMIC driver that sets VDDARM to
>> minimum possible voltage that is LARGER THAN or EQUAL TO max_vol, not
>> min_vol. I'll correct set_voltage usage of CPUFREQ assuming that
>> bugfix for MAX8998 PMIC driver is sent. (Joonyoung Shim, who's just
>> started submitting MAX8998 PMIC driver, know about this issue and will
>> probably send a patch soon.)
>
> Oh, fail. ?Like you say that's a bug in the regulator driver which will
> hopefully be fixed before this gets merged.
>
>> Besides, the whole section enclosed by this CONFIG_REGULATOR will be removed.
>
>> A while ago, WM8994 had been using CLK_OUT as the clock source with
>> APLL dependent setting; thus "target" function accessed CLK_OUT when
>> APLL changed. However, it no longer uses APLL dependent clock source
>> and we no longer access CLK_OUT at the "target" function; thus, this
>> code in "init" function can be removed.
>
> Ah, OK. ?If other people use that sort of clocking scheme we will need
> to come up with a reasonable way of coping with it, but let's save that
> for when the problem arises.

Such people may use cpufreq_notify_transition(freq,
CPUFREQ_PRECHANGE/POSTCHANGE). Some display drivers have been using
this feature to adjust clock settings when APLL clock speed changes.
Sound drivers with CLK_OUT should've used that, too.

>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[Mark Brown]

On Fri, Jul 16, 2010 at 05:30:03PM +0900, MyungJoo Ham wrote:
> On Fri, Jul 16, 2010 at 5:17 PM, Mark Brown

> > ...the ramp times are usually vanishingly small so the overwhelming
> > majority of consumers probably don't care anyway. ?If it's a problem we
> > can add an interface which doesn't do the delay automatically but for
> > most users it's going to be simpler to just deal with it transparently.

> It's about 30us in this driver (100MHz -> 1GHz), where we lost about
> 60000 instructions (2 instructions / cycle @ 100MHz, with default RAMP
> UP of 10mV/us). However, let's assume that it's ok for now.

Is the ramp actually required for systems?  The obvious thought here is
that if the ramp time can be reduced or eliminated by configuring the
regulator it'd be better to do that.

> > Ah, OK. ?If other people use that sort of clocking scheme we will need
> > to come up with a reasonable way of coping with it, but let's save that
> > for when the problem arises.

> Such people may use cpufreq_notify_transition(freq,
> CPUFREQ_PRECHANGE/POSTCHANGE). Some display drivers have been using
> this feature to adjust clock settings when APLL clock speed changes.
> Sound drivers with CLK_OUT should've used that, too.

Depending on how the clocking is integrated it might need to be bound a
bit tighter to the CPU management code, though.  If it's supplying a
clock for the CPU then it may need the CPU to be quiesced before you can
alter the output.

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[MyungJoo Ham]

On Fri, Jul 16, 2010 at 5:42 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Jul 16, 2010 at 05:30:03PM +0900, MyungJoo Ham wrote:
>> On Fri, Jul 16, 2010 at 5:17 PM, Mark Brown
>
>> > ...the ramp times are usually vanishingly small so the overwhelming
>> > majority of consumers probably don't care anyway. ?If it's a problem we
>> > can add an interface which doesn't do the delay automatically but for
>> > most users it's going to be simpler to just deal with it transparently.
>
>> It's about 30us in this driver (100MHz -> 1GHz), where we lost about
>> 60000 instructions (2 instructions / cycle @ 100MHz, with default RAMP
>> UP of 10mV/us). However, let's assume that it's ok for now.
>
> Is the ramp actually required for systems? ?The obvious thought here is
> that if the ramp time can be reduced or eliminated by configuring the
> regulator it'd be better to do that.
>

It appears that the ramp is actually required. Without ramp, the
driver has no idea about the voltage change delay, which can be
hazardous when the supplied voltage is required to increase. For
example, when the CPU goes from 100MHz to 1GHz, the supplied voltage
goes from 0.95V to 1.25V. After setting the voltage to 1.25V from
0.95V, CPUFREQ sets the frequency to 1GHz. However, at 1GHz, CPU may
fail (and locks up) if the supplied voltage is lower than 1.25V
(regulator voltage is still climbing).

Yes, with RAMP off, the voltage goes up to 1.25V very fast (appears to
be less than 1mV/us in our hardware), but not fast enough to prevent
executing some (about a hundred or thousand?) instructions. Thus,
codes after setting the clock to 1GHz are executed while the supplied
voltage it not enough. Then, CPU may fail although the probability
wouldn't be high enough to be seen frequently.

RAMP feature makes this delay deterministic lets us predict the
behavior and prevent running at higher frequency when the voltage is
not stabilized.


>> > Ah, OK. ?If other people use that sort of clocking scheme we will need
>> > to come up with a reasonable way of coping with it, but let's save that
>> > for when the problem arises.
>
>> Such people may use cpufreq_notify_transition(freq,
>> CPUFREQ_PRECHANGE/POSTCHANGE). Some display drivers have been using
>> this feature to adjust clock settings when APLL clock speed changes.
>> Sound drivers with CLK_OUT should've used that, too.
>
> Depending on how the clocking is integrated it might need to be bound a
> bit tighter to the CPU management code, though. ?If it's supplying a
> clock for the CPU then it may need the CPU to be quiesced before you can
> alter the output.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[Mark Brown]

On Fri, Jul 16, 2010 at 06:01:51PM +0900, MyungJoo Ham wrote:
> On Fri, Jul 16, 2010 at 5:42 PM, Mark Brown
> > On Fri, Jul 16, 2010 at 05:30:03PM +0900, MyungJoo Ham wrote:

> >> It's about 30us in this driver (100MHz -> 1GHz), where we lost about
> >> 60000 instructions (2 instructions / cycle @ 100MHz, with default RAMP
> >> UP of 10mV/us). However, let's assume that it's ok for now.

> > Is the ramp actually required for systems? ?The obvious thought here is
> > that if the ramp time can be reduced or eliminated by configuring the
> > regulator it'd be better to do that.

> It appears that the ramp is actually required. Without ramp, the
> driver has no idea about the voltage change delay, which can be
> hazardous when the supplied voltage is required to increase. For

Sorry, I think you misunderstand - what I mean is that you say this is
the "default" ramp time which suggests that the ramp time can be
configured.  Hopefully one of the options available is to reduce it
which seems like a win overall since it avoids having to handle the ramp
in software and reduces the overall state change time.

> Yes, with RAMP off, the voltage goes up to 1.25V very fast (appears to
> be less than 1mV/us in our hardware), but not fast enough to prevent
> executing some (about a hundred or thousand?) instructions. Thus,
> codes after setting the clock to 1GHz are executed while the supplied
> voltage it not enough. Then, CPU may fail although the probability
> wouldn't be high enough to be seen frequently.

> RAMP feature makes this delay deterministic lets us predict the
> behavior and prevent running at higher frequency when the voltage is
> not stabilized.

Right, you will get a ramp time due to the need to charge the capacitors
on the output of the DCDC - this applies to pretty much all regulators -
but it will be very much reduced which does substantially mitigate the
effects which makes a simpler approach in software much less of a
problem.

With high performance regualtors the limit is pretty much entirely the
capacitor on the output, it should actually be predictable and suitable
for handling within the driver.

[PATCH 2/5] ARM: S5P: Added default pll values for APLL 800/1000MHz

[Kukjin Kim]

MyungJoo Ham wrote:
> 
> CPUFREQ of S5PV210 uses different APLL settings and we provide
> such values for CPUFREQ at pll.h. We have been using differently
> between EVT0 and EVT1 machines. Although this version of kernel
> assumes that the CPU is EVT1, users may use code for EVT0 later.
> 
> Note that at 1GHz of ARMCLK, APLL should be 1GHz and for other lower
> ARMCLK, APLL should be 800MHz.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/plat-s5p/include/plat/pll.h |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-s5p/include/plat/pll.h
b/arch/arm/plat-s5p/include/plat/pll.h
> index 7db3227..3112aba 100644
> --- a/arch/arm/plat-s5p/include/plat/pll.h
> +++ b/arch/arm/plat-s5p/include/plat/pll.h
> @@ -21,6 +21,14 @@
> 
>  #include <asm/div64.h>
> 
> +#ifdef CONFIG_CPU_S5PC110_EVT0_ERRATA

Actually, EVT0 is not real chip and not for mass production.
So don't use in here.

> +#define PLL45XX_APLL_VAL_1000	(1 << 31) | (0xfa<<16) | (0x6<<8) |
(0x1)
> +#define PLL45XX_APLL_VAL_800	(1 << 31) | (0xc8<<16) | (0x6<<8) | (0x1)
> +#else
> +#define PLL45XX_APLL_VAL_1000	(1 << 31) | (125<<16) | (3<<8) | (1)
> +#define PLL45XX_APLL_VAL_800	(1 << 31) | (100<<16) | (3<<8) | (1)
> +#endif
> +
>  enum pll45xx_type_t {
>  	pll_4500,
>  	pll_4502,
> --

And...I got the below result from scripts/checkpatch.pl...
If this time is first, maybe mistake...but this is not...

--

ERROR: Macros with complex values should be enclosed in parenthesis
#27: FILE: arch/arm/plat-s5p/include/plat/pll.h:25:
+#define PLL45XX_APLL_VAL_1000  (1 << 31) | (0xfa<<16) | (0x6<<8) | (0x1)

ERROR: Macros with complex values should be enclosed in parenthesis
#28: FILE: arch/arm/plat-s5p/include/plat/pll.h:26:
+#define PLL45XX_APLL_VAL_800   (1 << 31) | (0xc8<<16) | (0x6<<8) | (0x1)

ERROR: Macros with complex values should be enclosed in parenthesis
#30: FILE: arch/arm/plat-s5p/include/plat/pll.h:28:
+#define PLL45XX_APLL_VAL_1000  (1 << 31) | (125<<16) | (3<<8) | (1)

ERROR: Macros with complex values should be enclosed in parenthesis
#31: FILE: arch/arm/plat-s5p/include/plat/pll.h:29:
+#define PLL45XX_APLL_VAL_800   (1 << 31) | (100<<16) | (3<<8) | (1)

total: 4 errors, 0 warnings, 14 lines checked

--

I already said to you about that :-(
Please make sure your patch has no problem to send before submitting.

See the Documentation/SubmittingPatches...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[MyungJoo Ham]

On Fri, Jul 16, 2010 at 6:37 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Jul 16, 2010 at 06:01:51PM +0900, MyungJoo Ham wrote:
>> On Fri, Jul 16, 2010 at 5:42 PM, Mark Brown
>> > On Fri, Jul 16, 2010 at 05:30:03PM +0900, MyungJoo Ham wrote:
>
>> >> It's about 30us in this driver (100MHz -> 1GHz), where we lost about
>> >> 60000 instructions (2 instructions / cycle @ 100MHz, with default RAMP
>> >> UP of 10mV/us). However, let's assume that it's ok for now.
>
>> > Is the ramp actually required for systems? ?The obvious thought here is
>> > that if the ramp time can be reduced or eliminated by configuring the
>> > regulator it'd be better to do that.
>
>> It appears that the ramp is actually required. Without ramp, the
>> driver has no idea about the voltage change delay, which can be
>> hazardous when the supplied voltage is required to increase. For
>
> Sorry, I think you misunderstand - what I mean is that you say this is
> the "default" ramp time which suggests that the ramp time can be
> configured. ?Hopefully one of the options available is to reduce it
> which seems like a win overall since it avoids having to handle the ramp
> in software and reduces the overall state change time.

Uh oh.. I didn't mean that the default ramp time is the "required" or
"suggested" by the PMIC, but I meant that it is required by the CPU to
execute instructions without possible CPU lockups due to lower voltage
with a higher rate (that requires higher voltage). As long as the time
needed to increase the voltage to the required level is not zero, CPU
needs to wait for a while before increasing the rate and with ramp
time, we can make it deterministic. Without ramp time, we need to
assume that the delay with an estimation. In our hardware, we can also
turn off RAMP and let PMIC increase the voltage as soon as possible.
However, I'm sure that the delay is still larger than zero with a
chance of executing hundreds of instructions before the voltage
reaches the required value.

I don't mind how short a regulator can change the voltage. As long as
it is long enough to execute several instructions, it has some chance
to lock up a CPU if we do NOT wait for the change before increasing
the rate. And, the RAMP value guarantees the given wait time is enough
for the drivers.

>
>> Yes, with RAMP off, the voltage goes up to 1.25V very fast (appears to
>> be less than 1mV/us in our hardware), but not fast enough to prevent
>> executing some (about a hundred or thousand?) instructions. Thus,
>> codes after setting the clock to 1GHz are executed while the supplied
>> voltage it not enough. Then, CPU may fail although the probability
>> wouldn't be high enough to be seen frequently.
>
>> RAMP feature makes this delay deterministic lets us predict the
>> behavior and prevent running at higher frequency when the voltage is
>> not stabilized.
>
> Right, you will get a ramp time due to the need to charge the capacitors
> on the output of the DCDC - this applies to pretty much all regulators -
> but it will be very much reduced which does substantially mitigate the
> effects which makes a simpler approach in software much less of a
> problem.
>
> With high performance regualtors the limit is pretty much entirely the
> capacitor on the output, it should actually be predictable and suitable
> for handling within the driver.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[Mark Brown]

On Mon, Jul 19, 2010 at 09:40:24AM +0900, MyungJoo Ham wrote:

> Uh oh.. I didn't mean that the default ramp time is the "required" or
> "suggested" by the PMIC, but I meant that it is required by the CPU to
> execute instructions without possible CPU lockups due to lower voltage

You're still missing my point here.  What I'm saying is that by using
the slower default ramp rate of the PMIC you're making the length of the
stall in execution introduced by waiting for the ramp to complete more
severe than it needs to be, thus increasing the impact on the system and
the need to do work in software to mitigate that.  Reducing the ramp
time should result in improved system performance.

[PATCH 5/5] ARM: S5PV210: Initial CPUFREQ Support

[MyungJoo Ham]

On Mon, Jul 19, 2010 at 6:09 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Jul 19, 2010 at 09:40:24AM +0900, MyungJoo Ham wrote:
>
>> Uh oh.. I didn't mean that the default ramp time is the "required" or
>> "suggested" by the PMIC, but I meant that it is required by the CPU to
>> execute instructions without possible CPU lockups due to lower voltage
>
> You're still missing my point here. ?What I'm saying is that by using
> the slower default ramp rate of the PMIC you're making the length of the
> stall in execution introduced by waiting for the ramp to complete more
> severe than it needs to be, thus increasing the impact on the system and
> the need to do work in software to mitigate that. ?Reducing the ramp
> time should result in improved system performance.
>


Sure. reducing RAMP delay results in higher performance. I was only
stating that we need RAMP delay anyway. I'd let PMIC driver to set the
RAMP delay minimal or let the board file state the RAMP delay value
for the driver to initialise.


Thanks.


> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>



-- 
MyungJoo Ham (???), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858