On 08-06-15, 20:29, Pi-Cheng Chen wrote: Sorry for the delay, I have been quite busy recently. > +++ b/drivers/cpufreq/mt8173-cpufreq.c > +static LIST_HEAD(cpu_dvfs_info_list); > + > +static inline struct mtk_cpu_dvfs_info *to_mtk_cpu_dvfs_info( > + struct list_head *list) > +{ > + return list_entry(list, struct mtk_cpu_dvfs_info, node); > +} > + > +static inline void mtk_cpu_dvfs_info_add(struct mtk_cpu_dvfs_info *info) > +{ > + list_add(&info->node, &cpu_dvfs_info_list); > +} No stupid wrappers please. Doesn't make anything better. > + > +static struct mtk_cpu_dvfs_info *mtk_cpu_dvfs_info_get(int cpu) A very bad name to a routine with very specific functionality. > +{ > + struct mtk_cpu_dvfs_info *info; > + struct list_head *list; > + > + list_for_each(list, &cpu_dvfs_info_list) { > + info = to_mtk_cpu_dvfs_info(list); > + > + if (cpumask_test_cpu(cpu, info->cpus)) > + return info; > + } > + > + return NULL; > +} > + > +static void mtk_cpu_dvfs_info_release(void) > +{ > + struct list_head *list, *tmp; > + struct mtk_cpu_dvfs_info *info; > + > + list_for_each_safe(list, tmp, &cpu_dvfs_info_list) { > + info = to_mtk_cpu_dvfs_info(list); > + > + dev_pm_opp_free_cpufreq_table(info->cpu_dev, > + &info->freq_table); > + > + if (!IS_ERR(info->proc_reg)) > + regulator_put(info->proc_reg); > + if (!IS_ERR(info->sram_reg)) > + regulator_put(info->sram_reg); > + if (!IS_ERR(info->cpu_clk)) > + clk_put(info->cpu_clk); > + if (!IS_ERR(info->inter_clk)) > + clk_put(info->inter_clk); > + > + of_free_opp_table(info->cpu_dev); > + > + list_del(list); > + kfree(info); > + } > +} > + > +#define MIN(a, b) ((a) < (b) ? (a) : (b)) > +#define MAX(a, b) ((a) > (b) ? (a) : (b)) Look for these in kernel.h > +static int mtk_cpufreq_set_voltage(struct mtk_cpu_dvfs_info *info, int vproc) > +{ > + if (info->need_voltage_trace) > + return mtk_cpufreq_voltage_trace(info, vproc); > + else > + return regulator_set_voltage(info->proc_reg, vproc, > + vproc + VOLT_TOL); > +} > + > +static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > + unsigned int index) > +{ > + struct cpufreq_frequency_table *freq_table = policy->freq_table; > + struct clk *cpu_clk = policy->clk; > + struct clk *armpll = clk_get_parent(cpu_clk); > + struct mtk_cpu_dvfs_info *info = policy->driver_data; > + struct device *cpu_dev = info->cpu_dev; > + struct dev_pm_opp *opp; > + long freq_hz, old_freq_hz; > + int vproc, old_vproc, inter_vproc, target_vproc, ret; > + > + inter_vproc = info->intermediate_voltage; > + > + old_freq_hz = clk_get_rate(cpu_clk); > + old_vproc = regulator_get_voltage(info->proc_reg); > + > + freq_hz = freq_table[index].frequency * 1000; A blank line here. > + rcu_read_lock(); > + opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_hz); > + if (IS_ERR(opp)) { > + rcu_read_unlock(); > + pr_err("cpu%d: failed to find OPP for %ld\n", > + policy->cpu, freq_hz); > + return PTR_ERR(opp); > + } > + vproc = dev_pm_opp_get_voltage(opp); > + rcu_read_unlock(); > + > + /* > + * If the new voltage or the intermediate voltage is higher than the > + * current voltage, scale up voltage first. > + */ > + target_vproc = (inter_vproc > vproc) ? inter_vproc : vproc; > + if (old_vproc < target_vproc) { > + ret = mtk_cpufreq_set_voltage(info, target_vproc); > + if (ret) { > + pr_err("cpu%d: failed to scale up voltage!\n", > + policy->cpu); > + mtk_cpufreq_set_voltage(info, old_vproc); > + return ret; > + } > + } > + > + /* Reparent the CPU clock to intermediate clock. */ > + ret = clk_set_parent(cpu_clk, info->inter_clk); > + if (ret) { > + pr_err("cpu%d: failed to re-parent cpu clock!\n", > + policy->cpu); > + mtk_cpufreq_set_voltage(info, old_vproc); > + WARN_ON(1); > + return ret; > + } > + > + /* Set the original PLL to target rate. */ > + ret = clk_set_rate(armpll, freq_hz); > + if (ret) { > + pr_err("cpu%d: failed to scale cpu clock rate!\n", > + policy->cpu); > + clk_set_parent(cpu_clk, armpll); > + mtk_cpufreq_set_voltage(info, old_vproc); > + return ret; > + } > + > + /* Set parent of CPU clock back to the original PLL. */ > + ret = clk_set_parent(cpu_clk, armpll); > + if (ret) { > + pr_err("cpu%d: failed to re-parent cpu clock!\n", > + policy->cpu); > + mtk_cpufreq_set_voltage(info, inter_vproc); > + WARN_ON(1); > + return ret; > + } > + > + /* > + * If the new voltage is lower than the intermediate voltage or the > + * original voltage, scale down to the new voltage. > + */ > + if (vproc < inter_vproc || vproc < old_vproc) { > + ret = mtk_cpufreq_set_voltage(info, vproc); > + if (ret) { > + pr_err("cpu%d: failed to scale down voltage!\n", > + policy->cpu); > + clk_set_parent(cpu_clk, info->inter_clk); > + clk_set_rate(armpll, old_freq_hz); > + clk_set_parent(cpu_clk, armpll); > + return ret; > + } > + } > + > + return 0; > +} > + > +static int mtk_cpufreq_init(struct cpufreq_policy *policy) > +{ > + struct mtk_cpu_dvfs_info *info; > + int ret; > + > + info = mtk_cpu_dvfs_info_get(policy->cpu); > + if (!info) { > + pr_err("%s: mtk cpu dvfs info for cpu%d is not initialized\n", > + __func__, policy->cpu); > + return -ENODEV; > + } > + > + ret = cpufreq_table_validate_and_show(policy, info->freq_table); > + if (ret) { > + pr_err("%s: invalid frequency table: %d\n", __func__, ret); > + return ret; > + } > + > + cpumask_copy(policy->cpus, info->cpus); > + policy->driver_data = info; > + policy->clk = info->cpu_clk; > + > + return 0; > +} > + > +static struct cpufreq_driver mt8173_cpufreq_driver = { > + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK, > + .verify = cpufreq_generic_frequency_table_verify, > + .target_index = mtk_cpufreq_set_target, > + .get = cpufreq_generic_get, > + .init = mtk_cpufreq_init, > + .name = "mtk-cpufreq", > + .attr = cpufreq_generic_attr, > +}; > + > +static int mtk_cpu_dvfs_info_init(int cpu) > +{ > + struct device *cpu_dev; > + struct regulator *proc_reg = ERR_PTR(-ENODEV); > + struct regulator *sram_reg = ERR_PTR(-ENODEV); > + struct clk *cpu_clk = ERR_PTR(-ENODEV); > + struct clk *inter_clk = ERR_PTR(-ENODEV); > + struct mtk_cpu_dvfs_info *info; > + struct cpufreq_frequency_table *freq_table; > + struct dev_pm_opp *opp; > + unsigned long rate; > + int ret; > + > + cpu_dev = get_cpu_device(cpu); > + if (!cpu_dev) { > + pr_err("failed to get cpu%d device\n", cpu); > + return -ENODEV; > + } > + > + ret = of_init_opp_table(cpu_dev); > + if (ret) { > + pr_warn("no OPP table for cpu%d\n", cpu); > + return ret; > + } > + > + cpu_clk = clk_get(cpu_dev, "cpu"); > + if (IS_ERR(cpu_clk)) { > + if (PTR_ERR(cpu_clk) == -EPROBE_DEFER) > + pr_warn("cpu clk for cpu%d not ready, retry.\n", cpu); > + else > + pr_err("failed to get cpu clk for cpu%d\n", cpu); > + > + ret = PTR_ERR(cpu_clk); > + goto out_free_opp_table; > + } > + > + inter_clk = clk_get(cpu_dev, "intermediate"); > + if (IS_ERR(inter_clk)) { > + if (PTR_ERR(inter_clk) == -EPROBE_DEFER) > + pr_warn("intermediate clk for cpu%d not ready, retry.\n", > + cpu); > + else > + pr_err("failed to get intermediate clk for cpu%d\n", > + cpu); > + > + ret = PTR_ERR(cpu_clk); > + goto out_free_resources; > + } > + > + proc_reg = regulator_get_exclusive(cpu_dev, "proc"); > + if (IS_ERR(proc_reg)) { > + if (PTR_ERR(proc_reg) == -EPROBE_DEFER) > + pr_warn("proc regulator for cpu%d not ready, retry.\n", > + cpu); > + else > + pr_err("failed to get proc regulator for cpu%d\n", > + cpu); > + > + ret = PTR_ERR(proc_reg); > + goto out_free_resources; > + } > + > + /* Both presence and absence of sram regulator are valid cases. */ > + sram_reg = regulator_get_exclusive(cpu_dev, "sram"); > + > + info = kzalloc(sizeof(*info), GFP_KERNEL); > + if (!info) { > + ret = -ENOMEM; > + goto out_free_resources; > + } > + > + ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); > + if (ret) { > + pr_err("failed to init cpufreq table for cpu%d: %d\n", > + cpu, ret); > + goto out_free_mtk_cpu_dvfs_info; > + } > + > + if (!alloc_cpumask_var(&info->cpus, GFP_KERNEL)) Why getting into such trouble? What about just saving policy's pointer in info and use it for freq_table, cpus mask, etc ? > + goto out_free_cpufreq_table; > + > +} > + > +static int mt8173_cpufreq_probe(struct platform_device *pdev) > +{ > + int cpu, ret; > + > + for_each_possible_cpu(cpu) { > + /* > + * If the struct mtk_cpu_dvfs_info for the cpu power domain > + * is already initialized, skip this CPU. > + */ > + if (!mtk_cpu_dvfs_info_get(cpu)) { > + ret = mtk_cpu_dvfs_info_init(cpu); > + if (ret) { > + if (ret != -EPROBE_DEFER) > + pr_err("%s probe fail\n", __func__); > + > + mtk_cpu_dvfs_info_release(); > + return ret; > + } > + } > + } > + > + ret = cpufreq_register_driver(&mt8173_cpufreq_driver); > + if (ret) { > + pr_err("failed to register mtk cpufreq driver\n"); > + mtk_cpu_dvfs_info_release(); > + } > + > + return ret; > +} > + > +static struct platform_driver mt8173_cpufreq_platdrv = { > + .driver = { > + .name = "mt8173-cpufreq", > + }, > + .probe = mt8173_cpufreq_probe, > +}; > +module_platform_driver(mt8173_cpufreq_platdrv); > + > +static int mt8173_cpufreq_driver_init(void) > +{ > + struct platform_device *pdev; > + > + if (!of_machine_is_compatible("mediatek,mt8173")) > + return -ENODEV; > + > + pdev = platform_device_register_simple("mt8173-cpufreq", -1, NULL, 0); > + if (IS_ERR(pdev)) { > + pr_err("failed to register mtk-cpufreq platform device\n"); > + return PTR_ERR(pdev); > + } > + > + return 0; > +} > +module_init(mt8173_cpufreq_driver_init); Can this driver be built as module? Why this module* crap all over the place? -- viresh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in