Re: [PATCH v5 07/18] thermal: exynos: Modify exynos thermal code to use device tree for cpu cooling configuration

Viresh Kumar
2015-01-21T09:08:16Z

On 21 January 2015 at 14:03, Lukasz Majewski <l.majewski@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().

>> + 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 ?

>> + 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.