Hi Amit,<br><br>Please see my feedback embedded.<br><br><div class="gmail_quote">On Thu, Sep 30, 2010 at 6:48 PM, Amit Kucheria <span dir="ltr"><<a href="mailto:amit.kucheria@linaro.org">amit.kucheria@linaro.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Add'ed linaro-dev and linux-arm-kernel to CC.<br>
<br>
Thanks Yong, some feeback follows inline.<br>
<div><div></div><div class="h5"><br>
On 10 Sep 29, Yong Shen wrote:<br>
> From: Yong Shen <<a href="mailto:yong.shen@linaro.org">yong.shen@linaro.org</a>><br>
><br>
> ---<br>
> arch/arm/Kconfig | 6 +<br>
> arch/arm/mach-mx5/Kconfig | 1 +<br>
> arch/arm/mach-mx5/board-mx51_babbage.c | 32 ++++<br>
> arch/arm/mach-mx5/clock-mx51.c | 53 ++++++<br>
> arch/arm/plat-mxc/Makefile | 2 +<br>
> arch/arm/plat-mxc/cpufreq.c | 282 ++++++++++++++++++++++++++++++++<br>
> arch/arm/plat-mxc/include/mach/mxc.h | 20 +++<br>
> 7 files changed, 396 insertions(+), 0 deletions(-)<br>
> create mode 100644 arch/arm/plat-mxc/cpufreq.c<br>
><br>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig<br>
> index 4db064e..64ebbc0 100644<br>
> --- a/arch/arm/Kconfig<br>
> +++ b/arch/arm/Kconfig<br>
> @@ -1517,6 +1517,12 @@ if ARCH_HAS_CPUFREQ<br>
><br>
> source "drivers/cpufreq/Kconfig"<br>
><br>
> +config CPU_FREQ_IMX<br>
> + tristate "CPUfreq driver for i.MX CPUs"<br>
> + depends on ARCH_MXC && CPU_FREQ<br>
> + help<br>
> + This enables the CPUfreq driver for i.MX CPUs.<br>
> +<br>
> config CPU_FREQ_SA1100<br>
> bool<br>
><br>
> diff --git a/arch/arm/mach-mx5/Kconfig b/arch/arm/mach-mx5/Kconfig<br>
> index 1576d51..5956fee 100644<br>
> --- a/arch/arm/mach-mx5/Kconfig<br>
> +++ b/arch/arm/mach-mx5/Kconfig<br>
> @@ -5,6 +5,7 @@ config ARCH_MX51<br>
> default y<br>
> select MXC_TZIC<br>
> select ARCH_MXC_IOMUX_V3<br>
> + select ARCH_HAS_CPUFREQ<br>
><br>
> comment "MX5 platforms:"<br>
><br>
> diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c<br>
> index ed885f9..e449e0b 100644<br>
> --- a/arch/arm/mach-mx5/board-mx51_babbage.c<br>
> +++ b/arch/arm/mach-mx5/board-mx51_babbage.c<br>
> @@ -43,6 +43,31 @@<br>
> #define MX51_USB_PLL_DIV_19_2_MHZ 0x01<br>
> #define MX51_USB_PLL_DIV_24_MHZ 0x02<br>
><br>
> +struct cpu_wp *(*get_cpu_wp)(int *wp);<br>
> +static int num_cpu_wp = 2;<br>
<br>
</div></div>use sizeof(array) instead of hard coding this.<br></blockquote><div>yong: Agreed. <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im"><br>
> +/* working point(wp): 0 - 800MHz; 1 - 166.25MHz; */<br>
> +static struct cpu_wp cpu_wp_auto[] = {<br>
> + {<br>
> + .pll_rate = 800000000,<br>
> + .cpu_rate = 800000000,<br>
> + .pdf = 0,<br>
> + .mfi = 8,<br>
> + .mfd = 2,<br>
> + .mfn = 1,<br>
> + .cpu_podf = 0,<br>
> + .cpu_voltage = 1100000,},<br>
> + {<br>
> + .pll_rate = 800000000,<br>
> + .cpu_rate = 160000000,<br>
> + .pdf = 4,<br>
> + .mfi = 8,<br>
> + .mfd = 2,<br>
> + .mfn = 1,<br>
> + .cpu_podf = 4,<br>
> + .cpu_voltage = 850000,},<br>
> +};<br>
<br>
<br>
</div>This data should be moved out to a separate file (e.g. mx51_ratetable.h)<br>
since it will be useful to other boards too.<br>
<br>
If other boards can have different rate tables (and they can, depending on<br>
the revision of the silicon), then we can either 'assemble' the correct rate<br>
table based on a flag field or have explicit separate rate tables for each<br>
silicon revision.<br>
<br>
In any case, I suspect that there will be some core rates that will be common<br>
across silicon revisions.<br>
<div><div></div><div class="h5"><br></div></div></blockquote><div><br>Yong: We keep it in a board related file because we may have different voltage set to various boards. But like what you think, in most of case, we can use this table for different boards.<br>
<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><div class="h5">
> static struct platform_device *devices[] __initdata = {<br>
> &mxc_fec_device,<br>
> };<br>
> @@ -87,6 +112,12 @@ static struct imxuart_platform_data uart_pdata = {<br>
> .flags = IMXUART_HAVE_RTSCTS,<br>
> };<br>
><br>
> +struct cpu_wp *mx51_babbage_get_cpu_wp(int *wp)<br>
> +{<br>
> + *wp = num_cpu_wp;<br>
> + return cpu_wp_auto;<br>
> +}<br>
> +<br>
> static inline void mxc_init_imx_uart(void)<br>
> {<br>
> mxc_register_device(&mxc_uart_device0, &uart_pdata);<br>
> @@ -246,6 +277,7 @@ static void __init mxc_board_init(void)<br>
><br>
> static void __init mx51_babbage_timer_init(void)<br>
> {<br>
> + get_cpu_wp = mx51_babbage_get_cpu_wp;<br>
> mx51_clocks_init(32768, 24000000, 22579200, 0);<br>
> }<br>
><br>
> diff --git a/arch/arm/mach-mx5/clock-mx51.c b/arch/arm/mach-mx5/clock-mx51.c<br>
> index d9f612d..f2488e6 100644<br>
> --- a/arch/arm/mach-mx5/clock-mx51.c<br>
> +++ b/arch/arm/mach-mx5/clock-mx51.c<br>
> @@ -14,6 +14,7 @@<br>
> #include <linux/delay.h><br>
> #include <linux/clk.h><br>
> #include <linux/io.h><br>
> +#include <linux/time.h><br>
><br>
> #include <asm/clkdev.h><br>
> #include <asm/div64.h><br>
> @@ -28,6 +29,11 @@<br>
> static unsigned long external_high_reference, external_low_reference;<br>
> static unsigned long oscillator_reference, ckih2_reference;<br>
><br>
> +extern struct cpu_wp *(*get_cpu_wp)(int *wp);<br>
> +static int cpu_wp_nr;<br>
> +static int cpu_curr_wp;<br>
> +static struct cpu_wp *cpu_wp_tbl;<br>
> +<br>
> static struct clk osc_clk;<br>
> static struct clk pll1_main_clk;<br>
> static struct clk pll1_sw_clk;<br>
> @@ -38,7 +44,9 @@ static struct clk periph_apm_clk;<br>
> static struct clk ahb_clk;<br>
> static struct clk ipg_clk;<br>
> static struct clk usboh3_clk;<br>
> +static void __iomem *pll1_base;<br>
><br>
> +#define SPIN_DELAY 1000000 /* in nanoseconds */<br>
> #define MAX_DPLL_WAIT_TRIES 1000 /* 1000 * udelay(1) = 1ms */<br>
><br>
> static int _clk_ccgr_enable(struct clk *clk)<br>
> @@ -330,6 +338,32 @@ static int _clk_lp_apm_set_parent(struct clk *clk, struct clk *parent)<br>
> return 0;<br>
> }<br>
><br>
> +/*!<br>
> + * Setup cpu clock based on working point.<br>
> + * @param wp cpu freq working point<br>
> + * @return 0 on success or error code on failure.<br>
> + */<br>
> +int cpu_clk_set_wp(int wp)<br>
> +{<br>
> + struct cpu_wp *p;<br>
> + u32 reg;<br>
> +<br>
> + if (wp == cpu_curr_wp)<br>
> + return 0;<br>
> +<br>
> + p = &cpu_wp_tbl[wp];<br>
> +<br>
> + /*use post divider to change freq<br>
> + */<br>
> + reg = __raw_readl(MXC_CCM_CACRR);<br>
> + reg &= ~MXC_CCM_CACRR_ARM_PODF_MASK;<br>
> + reg |= cpu_wp_tbl[wp].cpu_podf << MXC_CCM_CACRR_ARM_PODF_OFFSET;<br>
> + __raw_writel(reg, MXC_CCM_CACRR);<br>
> + cpu_curr_wp = wp;<br>
> +<br>
> + return 0;<br>
> +}<br>
> +<br>
> static unsigned long clk_arm_get_rate(struct clk *clk)<br>
> {<br>
> u32 cacrr, div;<br>
> @@ -342,6 +376,20 @@ static unsigned long clk_arm_get_rate(struct clk *clk)<br>
> return parent_rate / div;<br>
> }<br>
><br>
> +int _clk_cpu_set_rate(struct clk *clk, unsigned long rate)<br>
> +{<br>
> + u32 i;<br>
> + for (i = 0; i < cpu_wp_nr; i++) {<br>
> + if (rate == cpu_wp_tbl[i].cpu_rate)<br>
> + break;<br>
> + }<br>
> + if (i >= cpu_wp_nr)<br>
> + return -EINVAL;<br>
> + cpu_clk_set_wp(i);<br>
> +<br>
> + return 0;<br>
> +}<br>
> +<br>
> static int _clk_periph_apm_set_parent(struct clk *clk, struct clk *parent)<br>
> {<br>
> u32 reg, mux;<br>
> @@ -694,6 +742,7 @@ static struct clk periph_apm_clk = {<br>
> static struct clk cpu_clk = {<br>
> .parent = &pll1_sw_clk,<br>
> .get_rate = clk_arm_get_rate,<br>
> + .set_rate = _clk_cpu_set_rate,<br>
> };<br>
><br>
> static struct clk ahb_clk = {<br>
> @@ -821,6 +870,7 @@ static struct clk_lookup lookups[] = {<br>
> _REGISTER_CLOCK("mxc-ehci.1", "usb_ahb", ahb_clk)<br>
> _REGISTER_CLOCK("fsl-usb2-udc", "usb", usboh3_clk)<br>
> _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", ahb_clk)<br>
> + _REGISTER_CLOCK(NULL, "cpu_clk", cpu_clk)<br>
> };<br>
><br>
> static void clk_tree_init(void)<br>
> @@ -848,10 +898,13 @@ int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,<br>
> {<br>
> int i;<br>
><br>
> + pll1_base = ioremap(MX51_PLL1_BASE_ADDR, SZ_4K);<br>
> +<br>
> external_low_reference = ckil;<br>
> external_high_reference = ckih1;<br>
> ckih2_reference = ckih2;<br>
> oscillator_reference = osc;<br>
> + cpu_wp_tbl = get_cpu_wp(&cpu_wp_nr);<br>
><br>
> for (i = 0; i < ARRAY_SIZE(lookups); i++)<br>
> clkdev_add(&lookups[i]);<br>
> diff --git a/arch/arm/plat-mxc/Makefile b/arch/arm/plat-mxc/Makefile<br>
> index 895bc3c..c1bb400 100644<br>
> --- a/arch/arm/plat-mxc/Makefile<br>
> +++ b/arch/arm/plat-mxc/Makefile<br>
> @@ -17,6 +17,8 @@ obj-$(CONFIG_USB_EHCI_MXC) += ehci.o<br>
> obj-$(CONFIG_MXC_ULPI) += ulpi.o<br>
> obj-$(CONFIG_ARCH_MXC_AUDMUX_V1) += audmux-v1.o<br>
> obj-$(CONFIG_ARCH_MXC_AUDMUX_V2) += audmux-v2.o<br>
> +# CPU FREQ support<br>
> +obj-$(CONFIG_CPU_FREQ_IMX) += cpufreq.o<br>
> ifdef CONFIG_SND_IMX_SOC<br>
> obj-y += ssi-fiq.o<br>
> obj-y += ssi-fiq-ksym.o<br>
> diff --git a/arch/arm/plat-mxc/cpufreq.c b/arch/arm/plat-mxc/cpufreq.c<br>
> new file mode 100644<br>
> index 0000000..cae40f1<br>
> --- /dev/null<br>
> +++ b/arch/arm/plat-mxc/cpufreq.c<br>
> @@ -0,0 +1,282 @@<br>
> +/*<br>
> + * Copyright (C) 2010 Freescale Semiconductor, Inc. All Rights Reserved.<br>
> + */<br>
> +<br>
> +/*<br>
> + * The code contained herein is licensed under the GNU General Public<br>
> + * License. You may obtain a copy of the GNU General Public License<br>
> + * Version 2 or later at the following locations:<br>
> + *<br>
> + * <a href="http://www.opensource.org/licenses/gpl-license.html" target="_blank">http://www.opensource.org/licenses/gpl-license.html</a><br>
> + * <a href="http://www.gnu.org/copyleft/gpl.html" target="_blank">http://www.gnu.org/copyleft/gpl.html</a><br>
> + */<br>
> +<br>
> +/*!<br>
> + * @file cpufreq.c<br>
> + *<br>
> + * @brief A driver for the Freescale Semiconductor i.MXC CPUfreq module.<br>
> + *<br>
> + * The CPUFREQ driver is for controling CPU frequency. It allows you to change<br>
> + * the CPU clock speed on the fly.<br>
> + *<br>
> + * @ingroup PM<br>
> + */<br>
<br>
</div></div>Fix these comments to follow the kernel commenting style.<br></blockquote><div><br>Yong: OK <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="h5"><br>
> +#include <linux/types.h><br>
> +#include <linux/kernel.h><br>
> +#include <linux/cpufreq.h><br>
> +#include <linux/init.h><br>
> +#include <linux/proc_fs.h><br>
> +#include <linux/regulator/consumer.h><br>
> +#include <linux/clk.h><br>
> +#include <linux/delay.h><br>
> +#include <linux/io.h><br>
> +#include <mach/hardware.h><br>
> +#include <asm/setup.h><br>
> +#include <mach/clock.h><br>
> +#include <asm/cacheflush.h><br>
> +#include <linux/hrtimer.h><br>
> +<br>
> +int cpu_freq_khz_min;<br>
> +int cpu_freq_khz_max;<br>
> +int arm_lpm_clk;<br>
> +int arm_normal_clk;<br>
> +int cpufreq_suspended;<br>
> +int cpufreq_trig_needed;<br>
> +<br>
> +static struct clk *cpu_clk;<br>
> +static struct cpufreq_frequency_table imx_freq_table[4];<br>
> +<br>
> +extern int set_low_bus_freq(void);<br>
> +extern int set_high_bus_freq(int high_bus_speed);<br>
> +extern int low_freq_bus_used(void);<br>
> +<br>
> +extern struct cpu_wp *(*get_cpu_wp)(int *wp);<br>
> +static int cpu_wp_nr;<br>
> +static struct cpu_wp *cpu_wp_tbl;<br>
> +<br>
> +int set_cpu_freq(int freq)<br>
> +{<br>
> + int ret = 0;<br>
> + int org_cpu_rate;<br>
> + int gp_volt = 0;<br>
> + int i;<br>
> +<br>
> + org_cpu_rate = clk_get_rate(cpu_clk);<br>
> + if (org_cpu_rate == freq)<br>
> + return ret;<br>
> +<br>
> + for (i = 0; i < cpu_wp_nr; i++) {<br>
> + if (freq == cpu_wp_tbl[i].cpu_rate)<br>
> + gp_volt = cpu_wp_tbl[i].cpu_voltage;<br>
> + }<br>
> +<br>
> + if (gp_volt == 0)<br>
> + return ret;<br>
> +<br>
> + ret = clk_set_rate(cpu_clk, freq);<br>
> + if (ret != 0) {<br>
> + printk(KERN_DEBUG "cannot set CPU clock rate\n");<br>
> + return ret;<br>
> + }<br>
> +<br>
> + return ret;<br>
> +}<br>
> +<br>
> +static int mxc_verify_speed(struct cpufreq_policy *policy)<br>
> +{<br>
> + if (policy->cpu != 0)<br>
> + return -EINVAL;<br>
> +<br>
> + return cpufreq_frequency_table_verify(policy, imx_freq_table);<br>
> +}<br>
> +<br>
> +static unsigned int mxc_get_speed(unsigned int cpu)<br>
> +{<br>
> + if (cpu)<br>
> + return 0;<br>
> +<br>
> + return clk_get_rate(cpu_clk) / 1000;<br>
> +}<br>
> +<br>
> +static int calc_frequency_khz(int target, unsigned int relation)<br>
> +{<br>
> + int i;<br>
> +<br>
> + if ((target * 1000) == clk_get_rate(cpu_clk))<br>
> + return target;<br>
> +<br>
> + if (relation == CPUFREQ_RELATION_H) {<br>
> + for (i = cpu_wp_nr - 1; i >= 0; i--) {<br>
> + if (imx_freq_table[i].frequency <= target)<br>
> + return imx_freq_table[i].frequency;<br>
> + }<br>
> + } else if (relation == CPUFREQ_RELATION_L) {<br>
> + for (i = 0; i < cpu_wp_nr; i++) {<br>
> + if (imx_freq_table[i].frequency >= target)<br>
> + return imx_freq_table[i].frequency;<br>
> + }<br>
> + }<br>
> + printk(KERN_ERR "Error: No valid cpufreq relation\n");<br>
> + return cpu_freq_khz_max;<br>
> +}<br>
> +<br>
> +static int mxc_set_target(struct cpufreq_policy *policy,<br>
> + unsigned int target_freq, unsigned int relation)<br>
> +{<br>
> + struct cpufreq_freqs freqs;<br>
> + int freq_Hz;<br>
> + int ret = 0;<br>
> +<br>
> + if (cpufreq_suspended) {<br>
> + target_freq = clk_get_rate(cpu_clk) / 1000;<br>
> + freq_Hz = calc_frequency_khz(target_freq, relation) * 1000;<br>
> + if (freq_Hz == arm_lpm_clk)<br>
> + freqs.old = cpu_wp_tbl[cpu_wp_nr - 2].cpu_rate / 1000;<br>
> + else<br>
> + freqs.old = arm_lpm_clk / 1000;<br>
> +<br>
> + freqs.new = freq_Hz / 1000;<br>
> + freqs.cpu = 0;<br>
> + freqs.flags = 0;<br>
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);<br>
> + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);<br>
> + return ret;<br>
> + }<br>
> + /*<br>
> + * Some governors do not respects CPU and policy lower limits<br>
> + * which leads to bad things (division by zero etc), ensure<br>
> + * that such things do not happen.<br>
> + */<br>
<br>
</div></div>Isn't that a bug in the governor? Can you explain a bit?<br></blockquote><div>Yong: the original driver writer might have concern that some governor implementations will not care about the low limit suggested by cpu policy, therefore it is a change to correct them. <br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="h5"><br>
> + if (target_freq < policy->cpuinfo.min_freq)<br>
> + target_freq = policy->cpuinfo.min_freq;<br>
> +<br>
> + if (target_freq < policy->min)<br>
> + target_freq = policy->min;<br>
> +<br>
> + freq_Hz = calc_frequency_khz(target_freq, relation) * 1000;<br>
> +<br>
> + freqs.old = clk_get_rate(cpu_clk) / 1000;<br>
> + freqs.new = freq_Hz / 1000;<br>
> + freqs.cpu = 0;<br>
> + freqs.flags = 0;<br>
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);<br>
> +<br>
> + if (freqs.old != freqs.new)<br>
> + ret = set_cpu_freq(freq_Hz);<br>
> +<br>
> + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);<br>
> +<br>
> + return ret;<br>
> +}<br>
> +<br>
> +static int __init mxc_cpufreq_driver_init(struct cpufreq_policy *policy)<br>
> +{<br>
> + int ret;<br>
> + int i;<br>
> +<br>
> + printk(KERN_INFO "i.MXC CPU frequency driver\n");<br>
> +<br>
> + if (policy->cpu != 0)<br>
> + return -EINVAL;<br>
> +<br>
> + cpu_clk = clk_get(NULL, "cpu_clk");<br>
> + if (IS_ERR(cpu_clk)) {<br>
> + printk(KERN_ERR "%s: failed to get cpu clock\n", __func__);<br>
> + return PTR_ERR(cpu_clk);<br>
> + }<br>
> +<br>
> + /* Set the current working point. */<br>
> + cpu_wp_tbl = get_cpu_wp(&cpu_wp_nr);<br>
> +<br>
> + cpu_freq_khz_min = cpu_wp_tbl[0].cpu_rate / 1000;<br>
> + cpu_freq_khz_max = cpu_wp_tbl[0].cpu_rate / 1000;<br>
> +<br>
> + for (i = 0; i < cpu_wp_nr; i++) {<br>
> + imx_freq_table[cpu_wp_nr - 1 - i].index = cpu_wp_nr - i;<br>
<br>
</div></div>cpu_wp_nr = 2 here<br>
<br>
1st iteration of for loop:<br>
imx_freq_table[2 - 1 - 0].index = 2 - 0<br>
so, imx_freq_table[1].index = 2<br>
<br>
2nd iteration:<br>
imx_freq_table[2 - 1 - 1].index = 2 - 1<br>
imx_freq_table[0].index = 1<br>
<br>
So you're trying to reverse the table order? Why not just sort the table date<br>
in the way you want and add a comment on the top to keep it sorted.<br></blockquote><div>Yong: my understanding is that the freq table defined in another file is sorted in descending order, so the writer tends to make imx_freq_table in a descending order. <br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im"><br>
> + imx_freq_table[cpu_wp_nr - 1 - i].frequency =<br>
> + cpu_wp_tbl[i].cpu_rate / 1000;<br>
> +<br>
> + if ((cpu_wp_tbl[i].cpu_rate / 1000) < cpu_freq_khz_min)<br>
> + cpu_freq_khz_min = cpu_wp_tbl[i].cpu_rate / 1000;<br>
> +<br>
> + if ((cpu_wp_tbl[i].cpu_rate / 1000) > cpu_freq_khz_max)<br>
> + cpu_freq_khz_max = cpu_wp_tbl[i].cpu_rate / 1000;<br>
> + }<br>
> +<br>
> + imx_freq_table[i].index = 0;<br>
<br>
</div>imx_freq_table[i].index = i ?<br></blockquote><div>Yong: this should be a place holder, no meaning. <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im"><br>
> + imx_freq_table[i].frequency = CPUFREQ_TABLE_END;<br>
> +<br>
> + policy->cur = clk_get_rate(cpu_clk) / 1000;<br>
> + policy->governor = CPUFREQ_DEFAULT_GOVERNOR;<br>
> + policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;<br>
> + policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;<br>
> +<br>
> + arm_lpm_clk = cpu_freq_khz_min * 1000;<br>
> + arm_normal_clk = cpu_freq_khz_max * 1000;<br>
> +<br>
> + /* Manual states, that PLL stabilizes in two CLK32 periods */<br>
> + policy->cpuinfo.transition_latency = 10;<br>
> +<br>
> + ret = cpufreq_frequency_table_cpuinfo(policy, imx_freq_table);<br>
> +<br>
> + if (ret < 0) {<br>
> + clk_put(cpu_clk);<br>
> + printk(KERN_ERR "%s: failed to register i.MXC CPUfreq\n",<br>
> + __func__);<br>
> + return ret;<br>
> + }<br>
> +<br>
> + cpufreq_frequency_table_get_attr(imx_freq_table, policy->cpu);<br>
> + return 0;<br>
> +}<br>
> +<br>
> +static int mxc_cpufreq_suspend(struct cpufreq_policy *policy,<br>
> + pm_message_t state)<br>
> +{<br>
> + return 0;<br>
> +}<br>
<br>
</div>Get rid of these, since you don't use them.<br>
<div class="im"><br></div></blockquote><div>yong:Ok about this and below <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="im">
> +static int mxc_cpufreq_resume(struct cpufreq_policy *policy)<br>
> +{<br>
> + return 0;<br>
> +}<br>
<br>
</div>Same here.<br>
<div class="im"><br>
> +static int mxc_cpufreq_driver_exit(struct cpufreq_policy *policy)<br>
> +{<br>
> + cpufreq_frequency_table_put_attr(policy->cpu);<br>
> +<br>
> + /* Reset CPU to 665MHz */<br>
> + set_cpu_freq(arm_normal_clk);<br>
> + clk_put(cpu_clk);<br>
> + return 0;<br>
> +}<br>
> +<br>
> +static struct cpufreq_driver mxc_driver = {<br>
> + .flags = CPUFREQ_STICKY,<br>
> + .verify = mxc_verify_speed,<br>
> + .target = mxc_set_target,<br>
> + .get = mxc_get_speed,<br>
> + .init = mxc_cpufreq_driver_init,<br>
<br>
</div>mxc_cpufreq_init is ok. Lose the driver.<br>
<div class="im"><br>
> + .exit = mxc_cpufreq_driver_exit,<br>
<br>
</div>same.<br>
<div class="im"><br>
> + .suspend = mxc_cpufreq_suspend,<br>
> + .resume = mxc_cpufreq_resume,<br>
<br>
</div>Get rid of suspend/resume<br>
<div><div></div><div class="h5"><br>
> + .name = "imx",<br>
> +};<br>
> +<br>
> +static int __devinit mxc_cpufreq_init(void)<br>
> +{<br>
> + return cpufreq_register_driver(&mxc_driver);<br>
> +}<br>
> +<br>
> +static void mxc_cpufreq_exit(void)<br>
> +{<br>
> + cpufreq_unregister_driver(&mxc_driver);<br>
> +}<br>
> +<br>
> +module_init(mxc_cpufreq_init);<br>
> +module_exit(mxc_cpufreq_exit);<br>
> +<br>
> +MODULE_AUTHOR("Freescale Semiconductor, Inc.");<br>
> +MODULE_DESCRIPTION("CPUfreq driver for i.MX");<br>
> +MODULE_LICENSE("GPL");<br>
> diff --git a/arch/arm/plat-mxc/include/mach/mxc.h b/arch/arm/plat-mxc/include/mach/mxc.h<br>
> index a790bf2..14003b9 100644<br>
> --- a/arch/arm/plat-mxc/include/mach/mxc.h<br>
> +++ b/arch/arm/plat-mxc/include/mach/mxc.h<br>
> @@ -133,6 +133,26 @@ extern unsigned int __mxc_cpu_type;<br>
> # define cpu_is_mxc91231() (0)<br>
> #endif<br>
><br>
> +#ifndef __ASSEMBLY__<br>
> +<br>
> +struct cpu_wp {<br>
> + u32 pll_reg;<br>
> + u32 pll_rate;<br>
> + u32 cpu_rate;<br>
> + u32 pdr0_reg;<br>
> + u32 pdf;<br>
> + u32 mfi;<br>
> + u32 mfd;<br>
> + u32 mfn;<br>
> + u32 cpu_voltage;<br>
> + u32 cpu_podf;<br>
> +};<br>
> +<br>
> +#ifndef CONFIG_ARCH_MX5<br>
> +struct cpu_wp *get_cpu_wp(int *wp);<br>
> +#endif<br>
> +#endif<br>
> +<br>
> #if defined(CONFIG_ARCH_MX3) || defined(CONFIG_ARCH_MX2)<br>
> /* These are deprecated, use mx[23][157]_setup_weimcs instead. */<br>
> #define CSCR_U(n) (IO_ADDRESS(WEIM_BASE_ADDR + n * 0x10))<br>
> --<br>
> 1.6.3.3<br>
><br>
</div></div></blockquote></div><br>