Hi Viresh, > On 21 January 2015 at 14:03, Lukasz Majewski <l.majew...@samsung.com> > wrote: > >> diff --git a/drivers/cpufreq/exynos-cpufreq.c > > >> static int exynos_cpufreq_probe(struct platform_device *pdev) > >> { > >> + struct device_node *cpus, *np; > >> int ret = -EINVAL; > >> > >> exynos_info = kzalloc(sizeof(*exynos_info), GFP_KERNEL); > >> @@ -198,9 +202,31 @@ static int exynos_cpufreq_probe(struct > >> platform_device *pdev) /* Done here as we want to capture boot > >> frequency */ locking_frequency = > >> clk_get_rate(exynos_info->cpu_clk) / 1000; > >> - if (!cpufreq_register_driver(&exynos_driver)) > >> - return 0; > >> + if (cpufreq_register_driver(&exynos_driver)) > > You should return the error returned by cpufreq_register_driver().
OK. > > >> + goto err; > >> > >> + cpus = of_find_node_by_path("/cpus"); > >> + if (!cpus) { > >> + pr_err("failed to find cpus node\n"); > >> + return -ENOENT; > >> + } > >> + > >> + for (np = of_get_next_child(cpus, NULL); np; > >> + of_node_put(np), np = of_get_next_child(cpus, np)) { > >> + if (of_find_property(np, "#cooling-cells", NULL)) { > > Shouldn't you be checking this just for cpu 0 ? In previous versions I've only checked for cpu 0. If you think that it is enough to explicitly check only for cpu 0 and forget about above "fail safe" code (when. e.g. CPU3 has defined cooling-cells), then I'm fine with it. > > >> + cdev = of_cpufreq_cooling_register(np, > >> + > >> cpu_present_mask); > >> + if (IS_ERR(cdev)) > >> + pr_err("running cpufreq without > >> cooling device: %ld\n", > >> + PTR_ERR(cdev)); > >> + break; > >> + } > >> + } > >> + of_node_put(np); > >> + of_node_put(cpus); > >> + > >> + return 0; > >> + err: > >> dev_err(&pdev->dev, "failed to register cpufreq driver\n"); > >> regulator_put(arm_regulator); > >> err_vdd_arm: > > > > Viresh, the above is a small part of the cpufreq related code, > > which is necessary for Exynos thermal rework to use device tree. > > > > It is NOT adding any new functionality, but preserves possibility to > > use cpufreq as a colling device. > > > > Normally I would exclude this part from this commit, but then I > > cannot assure that between commits no regression is slipping in. > > Mostly looks fine. Just few nits. > > But another important thing is to split this patch, so that there is > a separate patch for cpufreq driver. As I've mention - it would be maintainer's call if one trades potential regression for patch separation. Thanks for a prompt reply. -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html