On Mon, Apr 22, 2013 at 08:55:35AM +0530, Viresh Kumar wrote:

On Fri, Apr 19, 2013 at 4:17 PM, Zhao Chenhui
[off-list ref] wrote:

quoted

diff --git a/drivers/cpufreq/mpc85xx-cpufreq.c b/drivers/cpufreq/mpc85xx-cpufreq.c

quoted

+#include <linux/module.h>
+#include <linux/cpufreq.h>
+#include <linux/of_platform.h>
+#include <linux/suspend.h>
+#include <linux/cpu.h>
+#include <linux/time.h>
+#include <linux/io.h>
+#include <linux/smp.h>

Would be better to keep them in alphabetical order, so that we don't add
anything twice.

Good idea.

quoted

+static int mpc85xx_cpufreq_cpu_init(struct cpufreq_policy *policy)
+{
+       unsigned int i, cur_pll;
+       int hw_cpu = get_hard_smp_processor_id(policy->cpu);
+
+       if (!cpu_present(policy->cpu))

This can't happen and so no need to check it.

quoted

+               return -ENODEV;
+
+       /* the latency of a transition, the unit is ns */
+       policy->cpuinfo.transition_latency = 2000;
+
+       cur_pll = get_pll(hw_cpu);
+
+       /* initialize frequency table */
+       pr_debug("core%d frequency table:\n", hw_cpu);
+       for (i = 0; mpc85xx_freqs[i].frequency != CPUFREQ_TABLE_END; i++) {
+               if (mpc85xx_freqs[i].index <= max_pll[hw_cpu]) {
+                       /* The frequency unit is kHz. */
+                       mpc85xx_freqs[i].frequency =
+                               (sysfreq * mpc85xx_freqs[i].index / 2) / 1000;
+               } else {
+                       mpc85xx_freqs[i].frequency = CPUFREQ_ENTRY_INVALID;
+               }
+
+               pr_debug("%d: %dkHz\n", i, mpc85xx_freqs[i].frequency);
+
+               if (mpc85xx_freqs[i].index == cur_pll)
+                       policy->cur = mpc85xx_freqs[i].frequency;
+       }
+       pr_debug("current pll is at %d, and core freq is%d\n",
+                       cur_pll, policy->cur);
+
+       cpufreq_frequency_table_get_attr(mpc85xx_freqs, policy->cpu);
+
+       /*
+        * This ensures that policy->cpuinfo_min
+        * and policy->cpuinfo_max are set correctly.
+        */
+       return cpufreq_frequency_table_cpuinfo(policy, mpc85xx_freqs);

Call cpufreq_frequency_table_get_attr() at the end after above call is
successful.

quoted

+}

quoted

+static int mpc85xx_cpufreq_target(struct cpufreq_policy *policy,
+                             unsigned int target_freq,
+                             unsigned int relation)

merge above two lines.

quoted

+{
+       struct cpufreq_freqs freqs;
+       unsigned int new;
+       int ret = 0;
+
+       if (!set_pll)
+               return -ENODEV;
+
+       cpufreq_frequency_table_target(policy,
+                                      mpc85xx_freqs,
+                                      target_freq,
+                                      relation,
+                                      &new);

same.. merge all above to put it in a single line.

quoted

+       freqs.old = policy->cur;
+       freqs.new = mpc85xx_freqs[new].frequency;
+       freqs.cpu = policy->cpu;

not required now.

quoted

+       mutex_lock(&mpc85xx_switch_mutex);
+       cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);

ditto. Rebase over latest code from linux-next. This call has changed.

quoted

+       ret = set_pll(policy->cpu, mpc85xx_freqs[new].index);
+       if (!ret) {
+               pr_info("cpufreq: Setting core%d frequency to %d kHz and PLL ratio to %d:2\n",
+                        policy->cpu, mpc85xx_freqs[new].frequency,
+                        mpc85xx_freqs[new].index);
+
+               ppc_proc_freq = freqs.new * 1000ul;
+       }
+       cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+       mutex_unlock(&mpc85xx_switch_mutex);
+
+       return ret;
+}

quoted

+static int __init mpc85xx_jog_init(void)
+{
+       struct device_node *np;
+       unsigned int svr;
+
+       np = of_find_matching_node(NULL, mpc85xx_jog_ids);
+       if (!np)
+               return -ENODEV;
+
+       guts = of_iomap(np, 0);
+       if (!guts) {
+               of_node_put(np);
+               return -ENODEV;
+       }
+
+       sysfreq = fsl_get_sys_freq();
+
+       if (of_device_is_compatible(np, "fsl,mpc8536-guts")) {
+               svr = mfspr(SPRN_SVR);
+               if ((svr & 0x7fff) == 0x10) {
+                       pr_err("MPC8536 Rev 1.0 does not support cpufreq(JOG).\n");
+                       of_node_put(np);

unmap too??

quoted

+                       return -ENODEV;
+               }
+               mpc85xx_freqs = mpc8536_freqs_table;
+               set_pll = mpc8536_set_pll;
+               max_pll[0] = get_pll(0);
+
+       } else if (of_device_is_compatible(np, "fsl,p1022-guts")) {
+               mpc85xx_freqs = p1022_freqs_table;
+               set_pll = p1022_set_pll;
+               max_pll[0] = get_pll(0);
+               max_pll[1] = get_pll(1);
+       }
+
+       pr_info("Freescale MPC85xx cpufreq(JOG) driver\n");
+
+       of_node_put(np);
+       return cpufreq_register_driver(&mpc85xx_cpufreq_driver);
+}
+
+device_initcall(mpc85xx_jog_init);

Thanks. I will fix them.

-Chenhui