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@xxxxxxxxxxx> > --- > 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 ? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html