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 <mma...@broadcom.com>");
> +MODULE_DESCRIPTION("CPUfreq driver for Broadcom BMIPS SoCs");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
--
viresh
