[PATCH 0/3] ASV support for Exynos4 series

[PATCH 0/3] ASV support for Exynos4 series

[Jongpill Lee]

[PATCH 1/3] ARM: EXYNOS: Add ASV feature for Exynos4 series
[PATCH 2/3] ARM: EXYNOS: Add clock for Exynos4210 asv feature
[PATCH 3/3] ARM: EXYNOS: Support ASV for Exynos4210

[PATCH 1/3] ARM: EXYNOS: Add ASV feature for Exynos4 series

[Jongpill Lee]

This patch adds ASV feature for exynos4 seires.
Asv(Adoptive support voltage) feature support to get spec of SoC.
And we can use to adjust voltage for operation SoC usign by ASV result.

Signed-off-by: Jongpill Lee <boyko.lee@samsung.com>
---
 arch/arm/mach-exynos4/Makefile           |    2 +-
 arch/arm/mach-exynos4/asv.c              |   78 ++++++++++++++++++++++++++++++
 arch/arm/mach-exynos4/include/mach/asv.h |   42 ++++++++++++++++
 3 files changed, 121 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-exynos4/asv.c
 create mode 100644 arch/arm/mach-exynos4/include/mach/asv.h

diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
index 2bb18f4..0f3affe 100644
--- a/arch/arm/mach-exynos4/Makefile
+++ b/arch/arm/mach-exynos4/Makefile
@@ -12,7 +12,7 @@ obj-				:=
 
 # Core support for EXYNOS4 system
 
-obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o
+obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o asv.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= setup-i2c0.o irq-eint.o dma.o pmu.o
 obj-$(CONFIG_CPU_EXYNOS4210)	+= clock-exynos4210.o
 obj-$(CONFIG_SOC_EXYNOS4212)	+= clock-exynos4212.o
diff --git a/arch/arm/mach-exynos4/asv.c b/arch/arm/mach-exynos4/asv.c
new file mode 100644
index 0000000..498fac0
--- /dev/null
+++ b/arch/arm/mach-exynos4/asv.c
@@ -0,0 +1,78 @@
+/* linux/arch/arm/mach-exynos4/asv.c
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * EXYNOS4 - ASV(Adaptive Supply Voltage) driver
+ *
+ * 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/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+
+#include <mach/map.h>
+#include <mach/asv.h>
+
+static struct samsung_asv *exynos_asv;
+
+static int __init exynos_asv_init(void)
+{
+	exynos_asv = kzalloc(sizeof(struct samsung_asv), GFP_KERNEL);
+	if (!exynos_asv)
+		goto out1;
+
+	/* I will add asv driver of exynos4 series to regist */
+
+	if (exynos_asv->check_vdd_arm) {
+		if (exynos_asv->check_vdd_arm()) {
+			pr_info("EXYNOS: It is wrong vdd_arm\n");
+			goto out2;
+		}
+	}
+
+	/* Get HPM Delay value */
+	if (exynos_asv->get_hpm) {
+		if (exynos_asv->get_hpm(exynos_asv)) {
+			pr_info("EXYNOS: Fail to get HPM Value\n");
+			goto out2;
+		}
+	} else {
+		pr_info("EXYNOS: There is no get hpm function\n");
+		goto out2;
+	}
+
+	/* Get IDS ARM Value */
+	if (exynos_asv->get_ids) {
+		if (exynos_asv->get_ids(exynos_asv)) {
+			pr_info("EXYNOS: Fail to get IDS Value\n");
+			goto out2;
+		}
+	} else {
+		pr_info("EXYNOS: There is no get ids function\n");
+		goto out2;
+	}
+
+	if (exynos_asv->store_result) {
+		if (exynos_asv->store_result(exynos_asv)) {
+			pr_info("EXYNOS: Can not success to store result\n");
+			goto out2;
+		}
+	} else {
+		pr_info("EXYNOS: No store_result function\n");
+		goto out2;
+	}
+
+	return 0;
+out2:
+	kfree(exynos_asv);
+out1:
+	return -EINVAL;
+}
+device_initcall_sync(exynos_asv_init);
diff --git a/arch/arm/mach-exynos4/include/mach/asv.h b/arch/arm/mach-exynos4/include/mach/asv.h
new file mode 100644
index 0000000..038a872
--- /dev/null
+++ b/arch/arm/mach-exynos4/include/mach/asv.h
@@ -0,0 +1,42 @@
+/* linux/arch/arm/mach-exynos4/include/mach/asv.h
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * EXYNOS4 - Adoptive Support Voltage Header file
+ *
+ * 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 __ASM_ARCH_ASV_H
+#define __ASM_ARCH_ASV_H __FILE__
+
+#define JUDGE_TABLE_END			NULL
+
+#define LOOP_CNT			10
+
+struct asv_judge_table {
+	unsigned int hpm_limit; /* HPM value to decide group of target */
+	unsigned int ids_limit; /* IDS value to decide group of target */
+};
+
+struct samsung_asv {
+	unsigned int pkg_id;			/* fused value for pakage */
+	unsigned int ids_offset;		/* ids_offset of chip */
+	unsigned int ids_mask;			/* ids_mask of chip */
+	unsigned int hpm_result;		/* hpm value of chip */
+	unsigned int ids_result;		/* ids value of chip */
+	int (*check_vdd_arm)(void);		/* check vdd_arm value, this function is selectable */
+	int (*pre_clock_init)(void);		/* clock init function to get hpm */
+	int (*pre_clock_setup)(void);		/* clock setup function to get hpm */
+	/* specific get ids function */
+	int (*get_ids)(struct samsung_asv *asv_info);
+	/* specific get hpm function */
+	int (*get_hpm)(struct samsung_asv *asv_info);
+	/* store into some repository to send result of asv */
+	int (*store_result)(struct samsung_asv *asv_info);
+};
+
+#endif /* __ASM_ARCH_ASV_H */
-- 
1.7.1

[PATCH 2/3] ARM: EXYNOS: Add clock for Exynos4210 asv feature

[Jongpill Lee]

This patch adds clock for exynos4210 asv feature
which name is sclk_pwi,sclk_hpm,dout_copy.
These clock is used for HPM devices

Signed-off-by: Jongpill Lee <boyko.lee@samsung.com>
---
 arch/arm/mach-exynos4/clock.c |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos4/clock.c b/arch/arm/mach-exynos4/clock.c
index db61691..230f851 100644
--- a/arch/arm/mach-exynos4/clock.c
+++ b/arch/arm/mach-exynos4/clock.c
@@ -306,6 +306,25 @@ static struct clksrc_clk clk_periphclk = {
 	.reg_div	= { .reg = S5P_CLKDIV_CPU, .shift = 12, .size = 3 },
 };
 
+static struct clk *clkset_mout_hpm_list[] = {
+	[0] = &clk_mout_apll.clk,
+	[1] = &clk_mout_mpll.clk,
+};
+
+static struct clksrc_sources clkset_sclk_hpm = {
+	.sources	= clkset_mout_hpm_list,
+	.nr_sources	= ARRAY_SIZE(clkset_mout_hpm_list),
+};
+
+static struct clksrc_clk clk_dout_copy = {
+	.clk	= {
+		.name		= "dout_copy",
+	},
+	.sources = &clkset_sclk_hpm,
+	.reg_src = { .reg = S5P_CLKSRC_CPU, .shift = 20, .size = 1 },
+	.reg_div = { .reg = S5P_CLKDIV_CPU1, .shift = 0, .size = 3 },
+};
+
 /* Core list of CMU_CORE side */
 
 struct clk *clkset_corebus_list[] = {
@@ -1239,7 +1258,20 @@ static struct clksrc_clk clksrcs[] = {
 			.ctrlbit	= (1 << 16),
 		},
 		.reg_div = { .reg = S5P_CLKDIV_FSYS3, .shift = 8, .size = 8 },
-	}
+	}, {
+		.clk	= {
+			.name		= "sclk_hpm",
+			.parent		= &clk_dout_copy.clk,
+		},
+		.reg_div = { .reg = S5P_CLKDIV_CPU1, .shift = 4, .size = 3 },
+	}, {
+		.clk	= {
+			.name		= "sclk_pwi",
+		},
+		.sources = &clkset_group,
+		.reg_src = { .reg = S5P_CLKSRC_DMC, .shift = 16, .size = 4 },
+		.reg_div = { .reg = S5P_CLKDIV_DMC1, .shift = 8, .size = 4 },
+	},
 };
 
 /* Clock initialization code */
@@ -1253,6 +1285,7 @@ static struct clksrc_clk *sysclks[] = {
 	&clk_armclk,
 	&clk_aclk_corem0,
 	&clk_aclk_cores,
+	&clk_dout_copy,
 	&clk_aclk_corem1,
 	&clk_periphclk,
 	&clk_mout_corebus,
-- 
1.7.1

[PATCH 3/3] ARM: EXYNOS: Support ASV for Exynos4210

[Jongpill Lee]

This patch adds function for exynos4210's asv driver.
On Exynos4210, we can get ASV result using by HPM and IDS value.
And asv result is sent through INFORM2 register.

Signed-off-by: Jongpill Lee <boyko.lee@samsung.com>
---
 arch/arm/mach-exynos4/Makefile                  |    2 +-
 arch/arm/mach-exynos4/asv-4210.c                |  338 +++++++++++++++++++++++
 arch/arm/mach-exynos4/asv.c                     |    8 +
 arch/arm/mach-exynos4/include/mach/asv.h        |    2 +
 arch/arm/mach-exynos4/include/mach/map.h        |    2 +
 arch/arm/mach-exynos4/include/mach/regs-clock.h |   18 ++
 arch/arm/mach-exynos4/include/mach/regs-iem.h   |   27 ++
 7 files changed, 396 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-exynos4/asv-4210.c
 create mode 100644 arch/arm/mach-exynos4/include/mach/regs-iem.h

diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
index 0f3affe..8627669 100644
--- a/arch/arm/mach-exynos4/Makefile
+++ b/arch/arm/mach-exynos4/Makefile
@@ -12,7 +12,7 @@ obj-				:=
 
 # Core support for EXYNOS4 system
 
-obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o asv.o
+obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o asv.o asv-4210.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= setup-i2c0.o irq-eint.o dma.o pmu.o
 obj-$(CONFIG_CPU_EXYNOS4210)	+= clock-exynos4210.o
 obj-$(CONFIG_SOC_EXYNOS4212)	+= clock-exynos4212.o
diff --git a/arch/arm/mach-exynos4/asv-4210.c b/arch/arm/mach-exynos4/asv-4210.c
new file mode 100644
index 0000000..81a1c67
--- /dev/null
+++ b/arch/arm/mach-exynos4/asv-4210.c
@@ -0,0 +1,338 @@
+/* linux/arch/arm/mach-exynos/asv-4210.c
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * EXYNOS4210 - ASV(Adaptive Supply Voltage) driver
+ *
+ * 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/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+
+#include <plat/clock.h>
+
+#include <mach/regs-iem.h>
+#include <mach/regs-clock.h>
+#include <mach/asv.h>
+
+enum target_asv {
+	EXYNOS4210_1200,
+	EXYNOS4210_1400,
+	EXYNOS4210_SINGLE_1200,
+};
+
+struct asv_judge_table exynos4210_1200_limit[] = {
+	/* HPM , IDS */
+	{8 , 4},
+	{11 , 8},
+	{14 , 12},
+	{18 , 17},
+	{21 , 27},
+	{23 , 45},
+	{25 , 55},
+};
+
+static struct asv_judge_table exynos4210_1400_limit[] = {
+	/* HPM , IDS */
+	{13 , 8},
+	{17 , 12},
+	{22 , 32},
+	{26 , 52},
+};
+
+static struct asv_judge_table exynos4210_single_1200_limit[] = {
+	/* HPM , IDS */
+	{8 , 4},
+	{14 , 12},
+	{21 , 27},
+	{25 , 55},
+};
+
+static int exynos4210_check_vdd_arm(void)
+{
+	/* It will be support */
+	return 0;
+}
+
+static int exynos4210_asv_pre_clock_init(void)
+{
+	struct clk *clk_hpm;
+	struct clk *clk_copy;
+	struct clk *clk_parent;
+
+	/* PWI clock setting */
+	clk_copy = clk_get(NULL, "sclk_pwi");
+	if (IS_ERR(clk_copy)) {
+		pr_info("EXYNOS4210: ASV : SCLK_PWI clock get error\n");
+		return -EINVAL;
+	} else {
+		clk_parent = clk_get(NULL, "xusbxti");
+
+		if (IS_ERR(clk_parent)) {
+			pr_info("EXYNOS4210: ASV: MOUT_APLL clock get error\n");
+			clk_put(clk_copy);
+			return -EINVAL;
+		}
+		if (clk_set_parent(clk_copy, clk_parent))
+			pr_info("EXYNOS4210: ASV: Unable to set parent %s of clock %s.\n",
+					clk_parent->name, clk_copy->name);
+
+		clk_put(clk_parent);
+	}
+	clk_set_rate(clk_copy, 4800000);
+
+	clk_put(clk_copy);
+
+	/* HPM clock setting */
+	clk_copy = clk_get(NULL, "dout_copy");
+	if (IS_ERR(clk_copy)) {
+		pr_info("EXYNOS4210: ASV: DOUT_COPY clock get error\n");
+		return -EINVAL;
+	} else {
+		clk_parent = clk_get(NULL, "mout_mpll");
+		if (IS_ERR(clk_parent)) {
+			pr_info("EXYNOS4210: ASV: MOUT_APLL clock get error\n");
+			clk_put(clk_copy);
+			return -EINVAL;
+		}
+		if (clk_set_parent(clk_copy, clk_parent))
+			pr_info("EXYNOS4210: ASV: Unable to set parent %s of clock %s.\n",
+					clk_parent->name, clk_copy->name);
+
+		clk_put(clk_parent);
+	}
+
+	clk_set_rate(clk_copy, (400 * 1000 * 1000));
+
+	clk_put(clk_copy);
+
+	clk_hpm = clk_get(NULL, "sclk_hpm");
+	if (IS_ERR(clk_hpm)) {
+		pr_info("EXYNOS4210: ASV: Fail to get sclk_hpm\n");
+		return -EINVAL;
+	}
+
+	clk_set_rate(clk_hpm, (200 * 1000 * 1000));
+
+	clk_put(clk_hpm);
+
+	return 0;
+}
+
+static int exynos4210_asv_pre_clock_setup(void)
+{
+	/* APLL_CON0 level register */
+	__raw_writel(0x80FA0601, S5P_APLL_CON0L8);
+	__raw_writel(0x80C80601, S5P_APLL_CON0L7);
+	__raw_writel(0x80C80602, S5P_APLL_CON0L6);
+	__raw_writel(0x80C80604, S5P_APLL_CON0L5);
+	__raw_writel(0x80C80601, S5P_APLL_CON0L4);
+	__raw_writel(0x80C80601, S5P_APLL_CON0L3);
+	__raw_writel(0x80C80601, S5P_APLL_CON0L2);
+	__raw_writel(0x80C80601, S5P_APLL_CON0L1);
+
+	/* IEM Divider register */
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L8);
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L7);
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L6);
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L5);
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L4);
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L3);
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L2);
+	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L1);
+
+	return 0;
+}
+
+static int exynos4210_find_group(struct samsung_asv *asv_info,
+			      enum target_asv exynos4_target)
+{
+	unsigned int ret = 0;
+	unsigned int i;
+
+	if (exynos4_target == EXYNOS4210_1200) {
+		ret = ARRAY_SIZE(exynos4210_1200_limit);
+
+		for (i = 0; i < ARRAY_SIZE(exynos4210_1200_limit); i++) {
+			if (asv_info->hpm_result <= exynos4210_1200_limit[i].hpm_limit ||
+			   asv_info->ids_result <= exynos4210_1200_limit[i].ids_limit) {
+				ret = i;
+				break;
+			}
+		}
+	} else if (exynos4_target == EXYNOS4210_1400) {
+		ret = ARRAY_SIZE(exynos4210_1400_limit);
+
+		for (i = 0; i < ARRAY_SIZE(exynos4210_1400_limit); i++) {
+			if (asv_info->hpm_result <= exynos4210_1400_limit[i].hpm_limit ||
+			   asv_info->ids_result <= exynos4210_1400_limit[i].ids_limit) {
+				ret = i;
+				break;
+			}
+		}
+	} else if (exynos4_target == EXYNOS4210_SINGLE_1200) {
+		ret = ARRAY_SIZE(exynos4210_single_1200_limit);
+
+		for (i = 0; i < ARRAY_SIZE(exynos4210_single_1200_limit); i++) {
+			if (asv_info->hpm_result <= exynos4210_single_1200_limit[i].hpm_limit ||
+			   asv_info->ids_result <= exynos4210_single_1200_limit[i].ids_limit) {
+				ret = i;
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
+#define PACK_ID				8
+#define PACK_MASK			0x3
+
+#define SUPPORT_1400MHZ			(1<<31)
+#define SUPPORT_1200MHZ			(1<<30)
+#define SUPPORT_1000MHZ			(1<<29)
+
+static int exynos4210_get_hpm(struct samsung_asv *asv_info)
+{
+	unsigned int i;
+	unsigned int tmp;
+	unsigned int hpm_delay = 0;
+	void __iomem *iem_base;
+
+	iem_base = ioremap(EXYNOS4_PA_IEM, (128 * 1024));
+
+	if (!iem_base) {
+		pr_info("EXYNOS: ioremap fail\n");
+		return -EPERM;
+	}
+
+	/* Clock setting to get asv value */
+	if (!asv_info->pre_clock_init) {
+		pr_info("EXYNOS: No Pre-setup function\n");
+		goto err;
+	} else {
+		if (asv_info->pre_clock_init()) {
+			pr_info("EXYNOS: pre_clock_init function fail");
+			goto err;
+		} else {
+			/* HPM enable  */
+			tmp = __raw_readl(iem_base + EXYNOS4_APC_CONTROL);
+			tmp |= APC_HPM_EN;
+			__raw_writel(tmp, (iem_base + EXYNOS4_APC_CONTROL));
+
+			asv_info->pre_clock_setup();
+
+			/* IEM enable */
+			tmp = __raw_readl(iem_base + EXYNOS4_IECDPCCR);
+			tmp |= IEC_EN;
+			__raw_writel(tmp, (iem_base + EXYNOS4_IECDPCCR));
+		}
+	}
+
+	/* Get HPM Delay value */
+	for (i = 0; i < LOOP_CNT; i++) {
+		tmp = __raw_readb(iem_base + EXYNOS4_APC_DBG_DLYCODE);
+		hpm_delay += tmp;
+	}
+
+	hpm_delay /= LOOP_CNT;
+
+	/* Store result of hpm value */
+	asv_info->hpm_result = hpm_delay;
+
+	return 0;
+
+err:
+	iounmap(iem_base);
+
+	return -EPERM;
+}
+
+static int exynos4210_get_ids(struct samsung_asv *asv_info)
+{
+	unsigned int pkg_id_val;
+
+	if (!asv_info->ids_offset || !asv_info->ids_mask) {
+		pr_info("EXYNOS4: No ids_offset or No ids_mask\n");
+		return -EPERM;
+	}
+
+	pkg_id_val = __raw_readl(S5P_VA_CHIPID + 0x4);
+	asv_info->pkg_id = pkg_id_val;
+	asv_info->ids_result = ((pkg_id_val >> asv_info->ids_offset) &
+							asv_info->ids_mask);
+
+	return 0;
+}
+
+static int exynos4210_asv_store_result(struct samsung_asv *asv_info)
+{
+	unsigned int result_grp;
+	char *support_freq;
+	unsigned int exynos_idcode = 0x0;
+
+	exynos_idcode = __raw_readl(S5P_VA_CHIPID);
+
+	/* Single chip is only support 1.2GHz */
+	if (!((exynos_idcode >> PACK_ID) & PACK_MASK)) {
+		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_SINGLE_1200);
+		result_grp |= SUPPORT_1200MHZ;
+		support_freq = "1.2GHz";
+
+		goto set_reg;
+	}
+
+	/* Check support freq */
+	switch (asv_info->pkg_id & 0x7) {
+	/* Support 1.2GHz */
+	case 1:
+	case 7:
+		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_1200);
+		result_grp |= SUPPORT_1200MHZ;
+		support_freq = "1.2GHz";
+		break;
+	/* Support 1.4GHz */
+	case 5:
+		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_1400);
+		result_grp |= SUPPORT_1200MHZ;
+		support_freq = "1.4GHz";
+		break;
+	/* Defalut support 1.0GHz */
+	default:
+		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_1200);
+		result_grp |= SUPPORT_1000MHZ;
+		support_freq = "1.0GHz";
+		break;
+	}
+
+set_reg:
+	__raw_writel(result_grp, S5P_INFORM2);
+
+	pr_info("Support %s\n", support_freq);
+	pr_info("ASV Group for This Exynos4210 is 0x%x\n", result_grp);
+
+	return 0;
+}
+
+void exynos4210_asv_init(struct samsung_asv *asv_info)
+{
+	pr_info("EXYNOS4210: Adaptive Support Voltage init\n");
+
+	asv_info->ids_offset = 24;
+	asv_info->ids_mask = 0xFF;
+
+	asv_info->get_ids = exynos4210_get_ids;
+	asv_info->get_hpm = exynos4210_get_hpm;
+	asv_info->check_vdd_arm = exynos4210_check_vdd_arm;
+	asv_info->pre_clock_init = exynos4210_asv_pre_clock_init;
+	asv_info->pre_clock_setup = exynos4210_asv_pre_clock_setup;
+	asv_info->store_result = exynos4210_asv_store_result;
+}
diff --git a/arch/arm/mach-exynos4/asv.c b/arch/arm/mach-exynos4/asv.c
index 498fac0..5acc8b1 100644
--- a/arch/arm/mach-exynos4/asv.c
+++ b/arch/arm/mach-exynos4/asv.c
@@ -17,6 +17,8 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 
+#include <plat/cpu.h>
+
 #include <mach/map.h>
 #include <mach/asv.h>
 
@@ -29,6 +31,12 @@ static int __init exynos_asv_init(void)
 		goto out1;
 
 	/* I will add asv driver of exynos4 series to regist */
+	if (soc_is_exynos4210())
+		exynos4210_asv_init(exynos_asv);
+	else {
+		pr_info("EXYNOS: No MACH ASV driver\n");
+		goto out2;
+	}
 
 	if (exynos_asv->check_vdd_arm) {
 		if (exynos_asv->check_vdd_arm()) {
diff --git a/arch/arm/mach-exynos4/include/mach/asv.h b/arch/arm/mach-exynos4/include/mach/asv.h
index 038a872..f84a11d 100644
--- a/arch/arm/mach-exynos4/include/mach/asv.h
+++ b/arch/arm/mach-exynos4/include/mach/asv.h
@@ -39,4 +39,6 @@ struct samsung_asv {
 	int (*store_result)(struct samsung_asv *asv_info);
 };
 
+extern void exynos4210_asv_init(struct samsung_asv *asv_info);
+
 #endif /* __ASM_ARCH_ASV_H */
diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
index 918a979..81fa158 100644
--- a/arch/arm/mach-exynos4/include/mach/map.h
+++ b/arch/arm/mach-exynos4/include/mach/map.h
@@ -60,6 +60,8 @@
 
 #define EXYNOS4_PA_COMBINER		0x10440000
 
+#define EXYNOS4_PA_IEM			0x10460000
+
 #define EXYNOS4_PA_GIC_CPU		0x10480000
 #define EXYNOS4_PA_GIC_DIST		0x10490000
 
diff --git a/arch/arm/mach-exynos4/include/mach/regs-clock.h b/arch/arm/mach-exynos4/include/mach/regs-clock.h
index 6c37ebe..23ee10f 100644
--- a/arch/arm/mach-exynos4/include/mach/regs-clock.h
+++ b/arch/arm/mach-exynos4/include/mach/regs-clock.h
@@ -130,6 +130,24 @@
 #define S5P_CLKGATE_SCLKCPU		S5P_CLKREG(0x14800)
 #define S5P_CLKGATE_IP_CPU		S5P_CLKREG(0x14900)
 
+#define S5P_APLL_CON0L8			S5P_CLKREG(0x15100)
+#define S5P_APLL_CON0L7			S5P_CLKREG(0x15104)
+#define S5P_APLL_CON0L6			S5P_CLKREG(0x15108)
+#define S5P_APLL_CON0L5			S5P_CLKREG(0x1510C)
+#define S5P_APLL_CON0L4			S5P_CLKREG(0x15110)
+#define S5P_APLL_CON0L3			S5P_CLKREG(0x15114)
+#define S5P_APLL_CON0L2			S5P_CLKREG(0x15118)
+#define S5P_APLL_CON0L1			S5P_CLKREG(0x1511C)
+
+#define S5P_CLKDIV_IEM_L8		S5P_CLKREG(0x15300)
+#define S5P_CLKDIV_IEM_L7		S5P_CLKREG(0x15304)
+#define S5P_CLKDIV_IEM_L6		S5P_CLKREG(0x15308)
+#define S5P_CLKDIV_IEM_L5		S5P_CLKREG(0x1530C)
+#define S5P_CLKDIV_IEM_L4		S5P_CLKREG(0x15310)
+#define S5P_CLKDIV_IEM_L3		S5P_CLKREG(0x15314)
+#define S5P_CLKDIV_IEM_L2		S5P_CLKREG(0x15318)
+#define S5P_CLKDIV_IEM_L1		S5P_CLKREG(0x1531C)
+
 #define S5P_APLL_LOCKTIME		(0x1C20)	/* 300us */
 
 #define S5P_APLLCON0_ENABLE_SHIFT	(31)
diff --git a/arch/arm/mach-exynos4/include/mach/regs-iem.h b/arch/arm/mach-exynos4/include/mach/regs-iem.h
new file mode 100644
index 0000000..d9bf177
--- /dev/null
+++ b/arch/arm/mach-exynos4/include/mach/regs-iem.h
@@ -0,0 +1,27 @@
+/* linux/arch/arm/mach-exynos/include/mach/regs-iem.h
+ *
+ * Copyright (c) 2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * EXYNOS4 - IEM(INTELLIGENT ENERGY MANAGEMENT) register discription
+ *
+ * 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 __ASM_ARCH_REGS_IEM_H
+#define __ASM_ARCH_REGS_IEM_H __FILE__
+
+/* Register for IEC  */
+#define EXYNOS4_IECDPCCR		(0x00000)
+
+/* Register for APC */
+#define EXYNOS4_APC_CONTROL		(0x10010)
+#define EXYNOS4_APC_PREDLYSEL		(0x10024)
+#define EXYNOS4_APC_DBG_DLYCODE		(0x100E0)
+
+#define APC_HPM_EN			(1 << 4)
+#define IEC_EN				(1 << 0)
+
+#endif /* __ASM_ARCH_REGS_IEM_H */
-- 
1.7.1

[PATCH 1/3] ARM: EXYNOS: Add ASV feature for Exynos4 series

[Sylwester Nawrocki]

Hi,

On 10/27/2011 03:12 AM, Jongpill Lee wrote:
> This patch adds ASV feature for exynos4 seires.

s/seires/series

> Asv(Adoptive support voltage) feature support to get spec of SoC.

So is this "Adoptive support voltage" or Adaptive Supply Voltage as commented
further in the patch ?

> And we can use to adjust voltage for operation SoC usign by ASV result.

s/usign/using

In general I find this commit description hard to understand. Could you please
provide a bit more detailed description, perhaps in the cover letter, of what 
this patch series does and why ?

> 
> Signed-off-by: Jongpill Lee<boyko.lee@samsung.com>
> ---
>   arch/arm/mach-exynos4/Makefile           |    2 +-
>   arch/arm/mach-exynos4/asv.c              |   78 ++++++++++++++++++++++++++++++
>   arch/arm/mach-exynos4/include/mach/asv.h |   42 ++++++++++++++++
>   3 files changed, 121 insertions(+), 1 deletions(-)
>   create mode 100644 arch/arm/mach-exynos4/asv.c
>   create mode 100644 arch/arm/mach-exynos4/include/mach/asv.h
> 
> diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
> index 2bb18f4..0f3affe 100644
> --- a/arch/arm/mach-exynos4/Makefile
> +++ b/arch/arm/mach-exynos4/Makefile
> @@ -12,7 +12,7 @@ obj-				:=
> 
>   # Core support for EXYNOS4 system
> 
> -obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o
> +obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o asv.o
>   obj-$(CONFIG_ARCH_EXYNOS4)	+= setup-i2c0.o irq-eint.o dma.o pmu.o
>   obj-$(CONFIG_CPU_EXYNOS4210)	+= clock-exynos4210.o
>   obj-$(CONFIG_SOC_EXYNOS4212)	+= clock-exynos4212.o
> diff --git a/arch/arm/mach-exynos4/asv.c b/arch/arm/mach-exynos4/asv.c
> new file mode 100644
> index 0000000..498fac0
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/asv.c
> @@ -0,0 +1,78 @@
> +/* linux/arch/arm/mach-exynos4/asv.c
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com/
> + *
> + * EXYNOS4 - ASV(Adaptive Supply Voltage) driver
> + *
> + * 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/init.h>
> +#include<linux/types.h>
> +#include<linux/kernel.h>
> +#include<linux/err.h>
> +#include<linux/io.h>
> +#include<linux/slab.h>
> +
> +#include<mach/map.h>
> +#include<mach/asv.h>
> +
> +static struct samsung_asv *exynos_asv;
> +
> +static int __init exynos_asv_init(void)
> +{
> +	exynos_asv = kzalloc(sizeof(struct samsung_asv), GFP_KERNEL);
> +	if (!exynos_asv)
> +		goto out1;

Just return -ENOMEM here.

> +
> +	/* I will add asv driver of exynos4 series to regist */

> +
> +	if (exynos_asv->check_vdd_arm) {
> +		if (exynos_asv->check_vdd_arm()) {
> +			pr_info("EXYNOS: It is wrong vdd_arm\n");
> +			goto out2;
> +		}
> +	}
> +
> +	/* Get HPM Delay value */
> +	if (exynos_asv->get_hpm) {
> +		if (exynos_asv->get_hpm(exynos_asv)) {
> +			pr_info("EXYNOS: Fail to get HPM Value\n");
> +			goto out2;
> +		}
> +	} else {
> +		pr_info("EXYNOS: There is no get hpm function\n");

Are you sure all these info printks are needed in the mainline kernel?

> +		goto out2;
> +	}
> +
> +	/* Get IDS ARM Value */
> +	if (exynos_asv->get_ids) {
> +		if (exynos_asv->get_ids(exynos_asv)) {
> +			pr_info("EXYNOS: Fail to get IDS Value\n");
> +			goto out2;
> +		}
> +	} else {
> +		pr_info("EXYNOS: There is no get ids function\n");
> +		goto out2;
> +	}
> +
> +	if (exynos_asv->store_result) {
> +		if (exynos_asv->store_result(exynos_asv)) {
> +			pr_info("EXYNOS: Can not success to store result\n");
> +			goto out2;
> +		}
> +	} else {
> +		pr_info("EXYNOS: No store_result function\n");
> +		goto out2;
> +	}
> +
> +	return 0;
> +out2:
> +	kfree(exynos_asv);
> +out1:
> +	return -EINVAL;
> +}
> +device_initcall_sync(exynos_asv_init);
> diff --git a/arch/arm/mach-exynos4/include/mach/asv.h b/arch/arm/mach-exynos4/include/mach/asv.h
> new file mode 100644
> index 0000000..038a872
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/include/mach/asv.h
> @@ -0,0 +1,42 @@
> +/* linux/arch/arm/mach-exynos4/include/mach/asv.h
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com/
> + *
> + * EXYNOS4 - Adoptive Support Voltage Header file
> + *
> + * 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 __ASM_ARCH_ASV_H
> +#define __ASM_ARCH_ASV_H __FILE__
> +
> +#define JUDGE_TABLE_END			NULL
> +
> +#define LOOP_CNT			10

These defines seem quite generic, if really needed perhaps it's worth to add
a prefix, e.g. EXYNOS4_. 

> +
> +struct asv_judge_table {
> +	unsigned int hpm_limit; /* HPM value to decide group of target */
> +	unsigned int ids_limit; /* IDS value to decide group of target */
> +};
> +
> +struct samsung_asv {

Wouldn't it be more appropriate to use more specific name, something like 'struct exynos_asv' ?

> +	unsigned int pkg_id;			/* fused value for pakage */
                                                                   ^^^^^^ 

> +	unsigned int ids_offset;		/* ids_offset of chip */
> +	unsigned int ids_mask;			/* ids_mask of chip */
> +	unsigned int hpm_result;		/* hpm value of chip */
> +	unsigned int ids_result;		/* ids value of chip */
> +	int (*check_vdd_arm)(void);		/* check vdd_arm value, this function is selectable */
> +	int (*pre_clock_init)(void);		/* clock init function to get hpm */
> +	int (*pre_clock_setup)(void);		/* clock setup function to get hpm */
> +	/* specific get ids function */
> +	int (*get_ids)(struct samsung_asv *asv_info);
> +	/* specific get hpm function */
> +	int (*get_hpm)(struct samsung_asv *asv_info);
> +	/* store into some repository to send result of asv */
> +	int (*store_result)(struct samsung_asv *asv_info);
> +};
> +
> +#endif /* __ASM_ARCH_ASV_H */

You're putting whole driver under arch/arm/, I'm wondering whether the functionality
it provides could be handled by some generic framework, like cpufreq or devfreq. 
How this driver is different from cpufreq drivers ?

--
Thanks,
Sylwester

[PATCH 3/3] ARM: EXYNOS: Support ASV for Exynos4210

[Sylwester Nawrocki]

On 10/27/2011 03:12 AM, Jongpill Lee wrote:
> This patch adds function for exynos4210's asv driver.
> On Exynos4210, we can get ASV result using by HPM and IDS value.
> And asv result is sent through INFORM2 register.
> 
> Signed-off-by: Jongpill Lee<boyko.lee@samsung.com>
> ---
>   arch/arm/mach-exynos4/Makefile                  |    2 +-
>   arch/arm/mach-exynos4/asv-4210.c                |  338 +++++++++++++++++++++++
>   arch/arm/mach-exynos4/asv.c                     |    8 +
>   arch/arm/mach-exynos4/include/mach/asv.h        |    2 +
>   arch/arm/mach-exynos4/include/mach/map.h        |    2 +
>   arch/arm/mach-exynos4/include/mach/regs-clock.h |   18 ++
>   arch/arm/mach-exynos4/include/mach/regs-iem.h   |   27 ++
>   7 files changed, 396 insertions(+), 1 deletions(-)
>   create mode 100644 arch/arm/mach-exynos4/asv-4210.c
>   create mode 100644 arch/arm/mach-exynos4/include/mach/regs-iem.h
> 
> diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
> index 0f3affe..8627669 100644
> --- a/arch/arm/mach-exynos4/Makefile
> +++ b/arch/arm/mach-exynos4/Makefile
> @@ -12,7 +12,7 @@ obj-				:=
> 
>   # Core support for EXYNOS4 system
> 
> -obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o asv.o
> +obj-$(CONFIG_ARCH_EXYNOS4)	+= cpu.o init.o clock.o irq-combiner.o asv.o asv-4210.o
>   obj-$(CONFIG_ARCH_EXYNOS4)	+= setup-i2c0.o irq-eint.o dma.o pmu.o
>   obj-$(CONFIG_CPU_EXYNOS4210)	+= clock-exynos4210.o
>   obj-$(CONFIG_SOC_EXYNOS4212)	+= clock-exynos4212.o
> diff --git a/arch/arm/mach-exynos4/asv-4210.c b/arch/arm/mach-exynos4/asv-4210.c
> new file mode 100644
> index 0000000..81a1c67
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/asv-4210.c
> @@ -0,0 +1,338 @@
> +/* linux/arch/arm/mach-exynos/asv-4210.c
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com/
> + *
> + * EXYNOS4210 - ASV(Adaptive Supply Voltage) driver
> + *
> + * 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/init.h>
> +#include<linux/types.h>
> +#include<linux/kernel.h>
> +#include<linux/err.h>
> +#include<linux/clk.h>
> +#include<linux/io.h>
> +
> +#include<plat/clock.h>
> +
> +#include<mach/regs-iem.h>
> +#include<mach/regs-clock.h>
> +#include<mach/asv.h>
> +
> +enum target_asv {
> +	EXYNOS4210_1200,
> +	EXYNOS4210_1400,
> +	EXYNOS4210_SINGLE_1200,
> +};
> +
> +struct asv_judge_table exynos4210_1200_limit[] = {
> +	/* HPM , IDS */
> +	{8 , 4},
> +	{11 , 8},
> +	{14 , 12},
> +	{18 , 17},
> +	{21 , 27},
> +	{23 , 45},
> +	{25 , 55},
> +};
> +
> +static struct asv_judge_table exynos4210_1400_limit[] = {
> +	/* HPM , IDS */
> +	{13 , 8},
> +	{17 , 12},
> +	{22 , 32},
> +	{26 , 52},
> +};
> +
> +static struct asv_judge_table exynos4210_single_1200_limit[] = {
> +	/* HPM , IDS */
> +	{8 , 4},
> +	{14 , 12},
> +	{21 , 27},
> +	{25 , 55},
> +};
> +
> +static int exynos4210_check_vdd_arm(void)
> +{
> +	/* It will be support */
> +	return 0;
> +}

If this function is empty might be better to have it removed now.

> +
> +static int exynos4210_asv_pre_clock_init(void)
> +{
> +	struct clk *clk_hpm;
> +	struct clk *clk_copy;
> +	struct clk *clk_parent;
> +
> +	/* PWI clock setting */
> +	clk_copy = clk_get(NULL, "sclk_pwi");
> +	if (IS_ERR(clk_copy)) {
> +		pr_info("EXYNOS4210: ASV : SCLK_PWI clock get error\n");
> +		return -EINVAL;

How about just doing: 
		return ERR_PTR(clk_copy);		
?

> +	} else {
> +		clk_parent = clk_get(NULL, "xusbxti");
> +
> +		if (IS_ERR(clk_parent)) {
> +			pr_info("EXYNOS4210: ASV: MOUT_APLL clock get error\n");
> +			clk_put(clk_copy);
> +			return -EINVAL;
> +		}
> +		if (clk_set_parent(clk_copy, clk_parent))
> +			pr_info("EXYNOS4210: ASV: Unable to set parent %s of clock %s.\n",

If I'm not mistaken, you could omit all occurrences of "EXYNOS4210: ASV: " string used with pr_<level>
functions if you have added at the beginning of this file:

#define pr_fmt(fmt) "EXYNOS4210: ASV: " fmt

> +					clk_parent->name, clk_copy->name);
> +
> +		clk_put(clk_parent);
> +	}
> +	clk_set_rate(clk_copy, 4800000);
> +
> +	clk_put(clk_copy);
> +
> +	/* HPM clock setting */
> +	clk_copy = clk_get(NULL, "dout_copy");
> +	if (IS_ERR(clk_copy)) {
> +		pr_info("EXYNOS4210: ASV: DOUT_COPY clock get error\n");
> +		return -EINVAL;
> +	} else {
> +		clk_parent = clk_get(NULL, "mout_mpll");
> +		if (IS_ERR(clk_parent)) {
> +			pr_info("EXYNOS4210: ASV: MOUT_APLL clock get error\n");
> +			clk_put(clk_copy);
> +			return -EINVAL;
> +		}
> +		if (clk_set_parent(clk_copy, clk_parent))
> +			pr_info("EXYNOS4210: ASV: Unable to set parent %s of clock %s.\n",
> +					clk_parent->name, clk_copy->name);
> +
> +		clk_put(clk_parent);
> +	}
> +
> +	clk_set_rate(clk_copy, (400 * 1000 * 1000));
> +
> +	clk_put(clk_copy);

The above looks like 2 copies of same thing. Ever considered making a function
out of one of these and using it here instead ?

> +
> +	clk_hpm = clk_get(NULL, "sclk_hpm");
> +	if (IS_ERR(clk_hpm)) {
> +		pr_info("EXYNOS4210: ASV: Fail to get sclk_hpm\n");
> +		return -EINVAL;
> +	}
> +
> +	clk_set_rate(clk_hpm, (200 * 1000 * 1000));
> +
> +	clk_put(clk_hpm);
> +
> +	return 0;
> +}
> +
> +static int exynos4210_asv_pre_clock_setup(void)
> +{
> +	/* APLL_CON0 level register */
> +	__raw_writel(0x80FA0601, S5P_APLL_CON0L8);
> +	__raw_writel(0x80C80601, S5P_APLL_CON0L7);
> +	__raw_writel(0x80C80602, S5P_APLL_CON0L6);
> +	__raw_writel(0x80C80604, S5P_APLL_CON0L5);
> +	__raw_writel(0x80C80601, S5P_APLL_CON0L4);
> +	__raw_writel(0x80C80601, S5P_APLL_CON0L3);
> +	__raw_writel(0x80C80601, S5P_APLL_CON0L2);
> +	__raw_writel(0x80C80601, S5P_APLL_CON0L1);
> +
> +	/* IEM Divider register */
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L8);
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L7);
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L6);
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L5);
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L4);
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L3);
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L2);
> +	__raw_writel(0x00500000, S5P_CLKDIV_IEM_L1);

hmm... lots of magic numbers here...

> +
> +	return 0;
> +}
> +
> +static int exynos4210_find_group(struct samsung_asv *asv_info,
> +			      enum target_asv exynos4_target)
> +{
> +	unsigned int ret = 0;
> +	unsigned int i;
> +
> +	if (exynos4_target == EXYNOS4210_1200) {
> +		ret = ARRAY_SIZE(exynos4210_1200_limit);
> +
> +		for (i = 0; i<  ARRAY_SIZE(exynos4210_1200_limit); i++) {
> +			if (asv_info->hpm_result<= exynos4210_1200_limit[i].hpm_limit ||
> +			   asv_info->ids_result<= exynos4210_1200_limit[i].ids_limit) {
> +				ret = i;
> +				break;

Perhaps it's just enough to return i directly.  

> +			}
> +		}
> +	} else if (exynos4_target == EXYNOS4210_1400) {
> +		ret = ARRAY_SIZE(exynos4210_1400_limit);
> +
> +		for (i = 0; i<  ARRAY_SIZE(exynos4210_1400_limit); i++) {
> +			if (asv_info->hpm_result<= exynos4210_1400_limit[i].hpm_limit ||
> +			   asv_info->ids_result<= exynos4210_1400_limit[i].ids_limit) {
> +				ret = i;
> +				break;
> +			}
> +		}
> +	} else if (exynos4_target == EXYNOS4210_SINGLE_1200) {
> +		ret = ARRAY_SIZE(exynos4210_single_1200_limit);
> +
> +		for (i = 0; i<  ARRAY_SIZE(exynos4210_single_1200_limit); i++) {
> +			if (asv_info->hpm_result<= exynos4210_single_1200_limit[i].hpm_limit ||
> +			   asv_info->ids_result<= exynos4210_single_1200_limit[i].ids_limit) {
> +				ret = i;
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +#define PACK_ID				8
> +#define PACK_MASK			0x3
> +
> +#define SUPPORT_1400MHZ			(1<<31)
> +#define SUPPORT_1200MHZ			(1<<30)
> +#define SUPPORT_1000MHZ			(1<<29)

? Do these happen to be S5P_INFORM2 register bit definitions ?
If so the names should tell it explicitly. And there are whitespaces
missing around '<<'.

> +
> +static int exynos4210_get_hpm(struct samsung_asv *asv_info)
> +{
> +	unsigned int i;
> +	unsigned int tmp;
> +	unsigned int hpm_delay = 0;
> +	void __iomem *iem_base;
> +
> +	iem_base = ioremap(EXYNOS4_PA_IEM, (128 * 1024));

More magic numbers...

> +
> +	if (!iem_base) {
> +		pr_info("EXYNOS: ioremap fail\n");

pr_info at an error level ?

> +		return -EPERM;
> +	}
> +
> +	/* Clock setting to get asv value */
> +	if (!asv_info->pre_clock_init) {
> +		pr_info("EXYNOS: No Pre-setup function\n");

ditto

> +		goto err;
> +	} else {

'else' is not needed

> +		if (asv_info->pre_clock_init()) {
> +			pr_info("EXYNOS: pre_clock_init function fail");
> +			goto err;

ditto

> +		} else {
> +			/* HPM enable  */
> +			tmp = __raw_readl(iem_base + EXYNOS4_APC_CONTROL);
> +			tmp |= APC_HPM_EN;
> +			__raw_writel(tmp, (iem_base + EXYNOS4_APC_CONTROL));
> +
> +			asv_info->pre_clock_setup();
> +
> +			/* IEM enable */
> +			tmp = __raw_readl(iem_base + EXYNOS4_IECDPCCR);
> +			tmp |= IEC_EN;
> +			__raw_writel(tmp, (iem_base + EXYNOS4_IECDPCCR));
> +		}
> +	}
> +
> +	/* Get HPM Delay value */
> +	for (i = 0; i<  LOOP_CNT; i++) {
> +		tmp = __raw_readb(iem_base + EXYNOS4_APC_DBG_DLYCODE);
> +		hpm_delay += tmp;
> +	}
> +
> +	hpm_delay /= LOOP_CNT;
> +
> +	/* Store result of hpm value */

This type of comments doesn't seem to add much value.

> +	asv_info->hpm_result = hpm_delay;
> +
> +	return 0;
> +
> +err:
> +	iounmap(iem_base);
> +
> +	return -EPERM;
> +}
> +
> +static int exynos4210_get_ids(struct samsung_asv *asv_info)
> +{
> +	unsigned int pkg_id_val;
> +
> +	if (!asv_info->ids_offset || !asv_info->ids_mask) {
> +		pr_info("EXYNOS4: No ids_offset or No ids_mask\n");
> +		return -EPERM;
> +	}
> +
> +	pkg_id_val = __raw_readl(S5P_VA_CHIPID + 0x4);
> +	asv_info->pkg_id = pkg_id_val;
> +	asv_info->ids_result = ((pkg_id_val>>  asv_info->ids_offset)&
> +							asv_info->ids_mask);
> +
> +	return 0;
> +}
> +
> +static int exynos4210_asv_store_result(struct samsung_asv *asv_info)
> +{
> +	unsigned int result_grp;
> +	char *support_freq;
> +	unsigned int exynos_idcode = 0x0;

Assigment to 0 is unneeded, since exynos_idcode is overwritten right below.
Also might be better to use u32 rather than 'unsigned int' in this case.

> +
> +	exynos_idcode = __raw_readl(S5P_VA_CHIPID);
> +
> +	/* Single chip is only support 1.2GHz */
> +	if (!((exynos_idcode>>  PACK_ID)&  PACK_MASK)) {
> +		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_SINGLE_1200);
> +		result_grp |= SUPPORT_1200MHZ;
> +		support_freq = "1.2GHz";
> +
> +		goto set_reg;
> +	}
> +
> +	/* Check support freq */

supported frequencies ?

> +	switch (asv_info->pkg_id&  0x7) {
> +	/* Support 1.2GHz */
> +	case 1:
> +	case 7:
> +		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_1200);
> +		result_grp |= SUPPORT_1200MHZ;
> +		support_freq = "1.2GHz";
> +		break;
> +	/* Support 1.4GHz */
> +	case 5:
> +		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_1400);
> +		result_grp |= SUPPORT_1200MHZ;

typo ?

> +		support_freq = "1.4GHz";
> +		break;
> +	/* Defalut support 1.0GHz */

s/Defalut/Default

I'm just wondering what exactly 'support' refers here to..

> +	default:
> +		result_grp = exynos4210_find_group(asv_info, EXYNOS4210_1200);
> +		result_grp |= SUPPORT_1000MHZ;
> +		support_freq = "1.0GHz";
> +		break;
> +	}
> +
> +set_reg:
> +	__raw_writel(result_grp, S5P_INFORM2);
> +
> +	pr_info("Support %s\n", support_freq);
> +	pr_info("ASV Group for This Exynos4210 is 0x%x\n", result_grp);
> +
> +	return 0;
> +}
> +
> +void exynos4210_asv_init(struct samsung_asv *asv_info)
> +{
> +	pr_info("EXYNOS4210: Adaptive Support Voltage init\n");
> +
> +	asv_info->ids_offset = 24;
> +	asv_info->ids_mask = 0xFF;

In the kernel lower case hex numbers are preferred.

> +
> +	asv_info->get_ids = exynos4210_get_ids;
> +	asv_info->get_hpm = exynos4210_get_hpm;
> +	asv_info->check_vdd_arm = exynos4210_check_vdd_arm;
> +	asv_info->pre_clock_init = exynos4210_asv_pre_clock_init;
> +	asv_info->pre_clock_setup = exynos4210_asv_pre_clock_setup;
> +	asv_info->store_result = exynos4210_asv_store_result;
> +}
> diff --git a/arch/arm/mach-exynos4/asv.c b/arch/arm/mach-exynos4/asv.c
> index 498fac0..5acc8b1 100644
> --- a/arch/arm/mach-exynos4/asv.c
> +++ b/arch/arm/mach-exynos4/asv.c
> @@ -17,6 +17,8 @@
>   #include<linux/io.h>
>   #include<linux/slab.h>
> 
> +#include<plat/cpu.h>
> +
>   #include<mach/map.h>
>   #include<mach/asv.h>
> 
> @@ -29,6 +31,12 @@ static int __init exynos_asv_init(void)
>   		goto out1;
> 
>   	/* I will add asv driver of exynos4 series to regist */

Is this comment still relevant ?

> +	if (soc_is_exynos4210())
> +		exynos4210_asv_init(exynos_asv);
> +	else {
> +		pr_info("EXYNOS: No MACH ASV driver\n");
> +		goto out2;
> +	}
> 
>   	if (exynos_asv->check_vdd_arm) {
>   		if (exynos_asv->check_vdd_arm()) {
> diff --git a/arch/arm/mach-exynos4/include/mach/asv.h b/arch/arm/mach-exynos4/include/mach/asv.h
> index 038a872..f84a11d 100644
> --- a/arch/arm/mach-exynos4/include/mach/asv.h
> +++ b/arch/arm/mach-exynos4/include/mach/asv.h
> @@ -39,4 +39,6 @@ struct samsung_asv {
>   	int (*store_result)(struct samsung_asv *asv_info);
>   };
> 
> +extern void exynos4210_asv_init(struct samsung_asv *asv_info);
> +
>   #endif /* __ASM_ARCH_ASV_H */
> diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
> index 918a979..81fa158 100644
> --- a/arch/arm/mach-exynos4/include/mach/map.h
> +++ b/arch/arm/mach-exynos4/include/mach/map.h
> @@ -60,6 +60,8 @@
> 
>   #define EXYNOS4_PA_COMBINER		0x10440000
> 
> +#define EXYNOS4_PA_IEM			0x10460000
> +
>   #define EXYNOS4_PA_GIC_CPU		0x10480000
>   #define EXYNOS4_PA_GIC_DIST		0x10490000
> 
> diff --git a/arch/arm/mach-exynos4/include/mach/regs-clock.h b/arch/arm/mach-exynos4/include/mach/regs-clock.h
> index 6c37ebe..23ee10f 100644
> --- a/arch/arm/mach-exynos4/include/mach/regs-clock.h
> +++ b/arch/arm/mach-exynos4/include/mach/regs-clock.h
> @@ -130,6 +130,24 @@
>   #define S5P_CLKGATE_SCLKCPU		S5P_CLKREG(0x14800)
>   #define S5P_CLKGATE_IP_CPU		S5P_CLKREG(0x14900)
> 
> +#define S5P_APLL_CON0L8			S5P_CLKREG(0x15100)
> +#define S5P_APLL_CON0L7			S5P_CLKREG(0x15104)
> +#define S5P_APLL_CON0L6			S5P_CLKREG(0x15108)
> +#define S5P_APLL_CON0L5			S5P_CLKREG(0x1510C)
> +#define S5P_APLL_CON0L4			S5P_CLKREG(0x15110)
> +#define S5P_APLL_CON0L3			S5P_CLKREG(0x15114)
> +#define S5P_APLL_CON0L2			S5P_CLKREG(0x15118)
> +#define S5P_APLL_CON0L1			S5P_CLKREG(0x1511C)

Do you think, instead of the above 8 lines, you could use something like:

#define S5P_APLL_CON0L(n)			S5P_CLKREG(0x15100 + (8 - (n)) * 4)

> +
> +#define S5P_CLKDIV_IEM_L8		S5P_CLKREG(0x15300)
> +#define S5P_CLKDIV_IEM_L7		S5P_CLKREG(0x15304)
> +#define S5P_CLKDIV_IEM_L6		S5P_CLKREG(0x15308)
> +#define S5P_CLKDIV_IEM_L5		S5P_CLKREG(0x1530C)
> +#define S5P_CLKDIV_IEM_L4		S5P_CLKREG(0x15310)
> +#define S5P_CLKDIV_IEM_L3		S5P_CLKREG(0x15314)
> +#define S5P_CLKDIV_IEM_L2		S5P_CLKREG(0x15318)
> +#define S5P_CLKDIV_IEM_L1		S5P_CLKREG(0x1531C)

and 

#define S5P_CLKDIV_IEM_L(n)		S5P_CLKREG(0x15300 + (8 - (n)) * 4)

(n = 1...8) ?

> +
>   #define S5P_APLL_LOCKTIME		(0x1C20)	/* 300us */
> 
>   #define S5P_APLLCON0_ENABLE_SHIFT	(31)
> diff --git a/arch/arm/mach-exynos4/include/mach/regs-iem.h b/arch/arm/mach-exynos4/include/mach/regs-iem.h
> new file mode 100644
> index 0000000..d9bf177
> --- /dev/null
> +++ b/arch/arm/mach-exynos4/include/mach/regs-iem.h
> @@ -0,0 +1,27 @@
> +/* linux/arch/arm/mach-exynos/include/mach/regs-iem.h
> + *
> + * Copyright (c) 2011 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com/
> + *
> + * EXYNOS4 - IEM(INTELLIGENT ENERGY MANAGEMENT) register discription
		    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^							 
Really need this in upper case ?

> + *
> + * 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 __ASM_ARCH_REGS_IEM_H
> +#define __ASM_ARCH_REGS_IEM_H __FILE__
> +
> +/* Register for IEC  */
> +#define EXYNOS4_IECDPCCR		(0x00000)
> +
> +/* Register for APC */
> +#define EXYNOS4_APC_CONTROL		(0x10010)
> +#define EXYNOS4_APC_PREDLYSEL		(0x10024)
> +#define EXYNOS4_APC_DBG_DLYCODE		(0x100E0)

Is there any specific reason to have parentheses around the numbers ?

[PATCH 3/3] ARM: EXYNOS: Support ASV for Exynos4210

[Russell King - ARM Linux]

On Thu, Oct 27, 2011 at 10:12:18AM +0900, Jongpill Lee wrote:
> +static int exynos4210_asv_pre_clock_init(void)
> +{
> +	struct clk *clk_hpm;
> +	struct clk *clk_copy;
> +	struct clk *clk_parent;
> +
> +	/* PWI clock setting */
> +	clk_copy = clk_get(NULL, "sclk_pwi");
> +	if (IS_ERR(clk_copy)) {
> +		pr_info("EXYNOS4210: ASV : SCLK_PWI clock get error\n");
> +		return -EINVAL;
> +	} else {
> +		clk_parent = clk_get(NULL, "xusbxti");
> +
> +		if (IS_ERR(clk_parent)) {
> +			pr_info("EXYNOS4210: ASV: MOUT_APLL clock get error\n");
> +			clk_put(clk_copy);
> +			return -EINVAL;
> +		}
> +		if (clk_set_parent(clk_copy, clk_parent))
> +			pr_info("EXYNOS4210: ASV: Unable to set parent %s of clock %s.\n",
> +					clk_parent->name, clk_copy->name);
> +
> +		clk_put(clk_parent);
> +	}
> +	clk_set_rate(clk_copy, 4800000);
> +
> +	clk_put(clk_copy);

This looks really weird - if something is using a clock, the reference
to the struct clk is supposed to be held all the time that the clock is
being used.  Plus there's nothing to enable the clock itself (so it's
probably disabled).

[PATCH 1/3] ARM: EXYNOS: Add ASV feature for Exynos4 series

[MyungJoo Ham]

On Thu, Oct 27, 2011 at 10:12 AM, Jongpill Lee <boyko.lee@samsung.com> wrote:
> This patch adds ASV feature for exynos4 seires.
> Asv(Adoptive support voltage) feature support to get spec of SoC.
> And we can use to adjust voltage for operation SoC usign by ASV result.
>
> Signed-off-by: Jongpill Lee <boyko.lee@samsung.com>
> ---
> ?arch/arm/mach-exynos4/Makefile ? ? ? ? ? | ? ?2 +-
> ?arch/arm/mach-exynos4/asv.c ? ? ? ? ? ? ?| ? 78 ++++++++++++++++++++++++++++++
> ?arch/arm/mach-exynos4/include/mach/asv.h | ? 42 ++++++++++++++++
> ?3 files changed, 121 insertions(+), 1 deletions(-)
> ?create mode 100644 arch/arm/mach-exynos4/asv.c
> ?create mode 100644 arch/arm/mach-exynos4/include/mach/asv.h
>

Hello,


As long as we have <mach/asv.h> in our hands, we can have the ASV
value stored at the common place declared by this asv.h rather than at
INFORM2 as I've mentioned before at another thread. This location
<mach/asv.h> seems the perfect location for the purpose. Besides, now,
with Exynos4x12s, I start to hear cases where all the INFORMx
registers are fully used; thus, it seems that it's time to remove
unnecessary usage of the registers.



Cheers!
MyungJoo

-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

[PATCH 1/3] ARM: EXYNOS: Add ASV feature for Exynos4 series

[MyungJoo Ham]

On Sat, Oct 29, 2011 at 3:48 AM, Sylwester Nawrocki <snjw23@gmail.com> wrote:
>
> You're putting whole driver under arch/arm/, I'm wondering whether the functionality
> it provides could be handled by some generic framework, like cpufreq or devfreq.
> How this driver is different from cpufreq drivers ?

I think there would be futher usage with ASV if IEM and proper PMICs
are attaches; however, for now, ASV guides proper vddarm and vddint
voltages for each freuquency.

Both drivers in cpufreq(vddarm and arm core freq) and devfreq(vddint
and memory/bus freq.. and GPU or MMC later?) should use the result
value of ASV because they both alter voltages according to the ASV
value.

Thus, if this is going to be located in drivers/, it'd better be in
elsewhere (drivers/misc? drivers/hwmon? ..???) and capable of
providing values to other device drivers, at least to cpufreq and
devfreq drivers.


Cheers!
MyungJoo

>
> --
> Thanks,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics