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.hu...@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().
