You must be a cpufreq driver expert by now. What's the count? Is this the 3rd one you have written ? :) On 01-02-17, 17:06, Markus Mayer wrote: > diff --git a/drivers/cpufreq/bmips-cpufreq.c b/drivers/cpufreq/bmips-cpufreq.c > +static struct cpufreq_frequency_table * > +bmips_cpufreq_get_freq_table(const struct cpufreq_policy *policy) Maybe call it bmips_cpufreq_create_freq_table() as that's what you are doing. But its all up to you only. > +{ > + struct cpufreq_frequency_table *table; > + struct cpufreq_compat *cc; > + unsigned long cpu_freq; > + int i; > + > + cc = policy->driver_data; > + cpu_freq = htp_freq_to_cpu_freq(cc->clk_mult); > + > + table = kzalloc((cc->max_freqs + 1) * sizeof(*table), GFP_KERNEL); Maybe kmalloc as you are updating all the entries. > + if (!table) > + return ERR_PTR(-ENOMEM); > + > + for (i = 0; i < cc->max_freqs; i++) { > + table[i].frequency = cpu_freq / (1 << i); > + table[i].driver_data = i; > + } > + table[i].frequency = CPUFREQ_TABLE_END; > + > + return table; > +} > + > +static unsigned int bmips_cpufreq_get(unsigned int cpu) > +{ > + struct cpufreq_policy *policy; > + struct cpufreq_compat *cc; > + unsigned long freq, cpu_freq; > + unsigned int div; > + uint32_t mode; > + > + policy = cpufreq_cpu_get(cpu); You need to do a corresponding cpufreq_cpu_put(). > + cc = policy->driver_data; > + > + switch (cc->bmips_type) { > + case BMIPS5200: > + case BMIPS5000: > + mode = read_c0_brcm_mode(); > + div = ((mode >> BMIPS5_CLK_DIV_SHIFT) & BMIPS5_CLK_DIV_MASK); > + break; > + default: > + div = 0; > + } > + > + cpu_freq = htp_freq_to_cpu_freq(cc->clk_mult); > + freq = cpu_freq / (1 << div); > + > + return freq; > +} > + > +static int bmips_cpufreq_target_index(struct cpufreq_policy *policy, > + unsigned int index) > +{ > + struct cpufreq_compat *cc; > + unsigned int div; > + > + cc = policy->driver_data; > + div = policy->freq_table[index].driver_data; > + > + switch (cc->bmips_type) { > + case BMIPS5200: > + case BMIPS5000: > + change_c0_brcm_mode(BMIPS5_CLK_DIV_MASK << BMIPS5_CLK_DIV_SHIFT, > + (1 << BMIPS5_CLK_DIV_SET_SHIFT) | > + (div << BMIPS5_CLK_DIV_SHIFT)); > + break; > + default: > + return -ENOTSUPP; > + } > + > + return 0; > +} > + > +static int bmips_cpufreq_exit(struct cpufreq_policy *policy) > +{ > + kfree(policy->freq_table); > + policy->freq_table = NULL; No need to set it to NULL. > + > + return 0; > +} > + > +static int bmips_cpufreq_init(struct cpufreq_policy *policy) > +{ > + struct cpufreq_frequency_table *freq_table; > + int ret; > + > + /* Store the compatibility data with the policy. */ > + policy->driver_data = cpufreq_get_driver_data(); Hmm, I wouldn't mind keeping a global variable for this. This driver will be probed only once and so we can simplify the code a bit. Up to you. > + > + freq_table = bmips_cpufreq_get_freq_table(policy); > + if (IS_ERR(freq_table)) { > + ret = PTR_ERR(freq_table); > + pr_err("%s: couldn't determine frequency table (%d).\n", > + BMIPS_CPUFREQ_NAME, ret); > + return ret; > + } > + > + ret = cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY); > + if (ret) > + bmips_cpufreq_exit(policy); > + else > + pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME); > + > + return ret; > +} > + > +static struct cpufreq_driver bmips_cpufreq_driver = { > + .flags = CPUFREQ_NEED_INITIAL_FREQ_CHECK, > + .verify = cpufreq_generic_frequency_table_verify, > + .target_index = bmips_cpufreq_target_index, > + .get = bmips_cpufreq_get, > + .init = bmips_cpufreq_init, > + .exit = bmips_cpufreq_exit, > + .attr = cpufreq_generic_attr, > + .name = BMIPS_CPUFREQ_PREFIX, > +}; > + > +static int __init bmips_cpufreq_probe(void) > +{ > + struct cpufreq_compat *cc; > + struct device_node *np; > + > + for (cc = bmips_cpufreq_compat; cc->compatible; cc++) { > + np = of_find_compatible_node(NULL, "cpu", cc->compatible); > + if (np) { > + of_node_put(np); > + bmips_cpufreq_driver.driver_data = cc; > + break; > + } > + } > + > + /* We hit the guard element of the array. No compatible CPU found. */ > + if (!cc->compatible) > + return -ENODEV; > + > + return cpufreq_register_driver(&bmips_cpufreq_driver); > +} > +device_initcall(bmips_cpufreq_probe); > + > +MODULE_AUTHOR("Markus Mayer <mmayer@xxxxxxxxxxxx>"); > +MODULE_DESCRIPTION("CPUfreq driver for Broadcom BMIPS SoCs"); > +MODULE_LICENSE("GPL"); > -- > 2.7.4 -- viresh