Messages in this thread | | | From | Viresh Kumar <> | Date | Tue, 20 Nov 2018 16:18:43 +0530 | Subject | Re: [PATCH] thermal: imx: fix for dependency on cpu-freq |
| |
While I am aligned with the fact that we need to carry this code for backward compatibility, there are few things I would suggest to improve.
On Wed, Oct 24, 2018 at 12:10 PM Anson Huang <anson.huang@nxp.com> wrote: > static int imx_thermal_probe(struct platform_device *pdev) > { > @@ -743,6 +745,7 @@ static int imx_thermal_probe(struct platform_device *pdev) > regmap_write(map, data->socdata->sensor_ctrl + REG_SET, > data->socdata->power_down_mask); > > +#ifdef CONFIG_CPU_FREQ > data->policy = cpufreq_cpu_get(0); > if (!data->policy) { > pr_debug("%s: CPUFreq policy not found\n", __func__); > @@ -755,6 +758,7 @@ static int imx_thermal_probe(struct platform_device *pdev) > "failed to register cpufreq cooling device: %d\n", ret); > return ret; > } > +#endif > > data->thermal_clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(data->thermal_clk)) {
You missed the error handling code which unregisters cooling/cpufreq stuff.
And it would be better to write things in a somewhat better way, like this:
#ifdef CONFIG_CPU_FREQ
static int imx_thermal_register_legacy_cooling(...) { ... current function body }
static void imx_thermal_unregister_legacy_cooling(...) { new routine body to unregister things }
#else static inline int imx_thermal_register_legacy_cooling(...) { return 0; }
static void imx_thermal_unregister_legacy_cooling(...) { }
#endif
And then you can get rid of ifdef hackery in the middle of probe().
| |