On Wed, May 16, 2018 at 5:11 PM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote: > On 16-05-18, 16:12, Amit Kucheria wrote: >> > + ret = PTR_ERR_OR_ZERO(opp_temp = >> > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); >> > + if (0 > ret) >> >> Any particular reason to prefer this over (ret < 0) that is generally >> used? I've seen it used to avoid the == vs. = typos, but not for other >> comparisons. >> >> Suggest sticking to what is commonly used i.e. ret < 0. >> >> > + goto free_opp; >> > + >> > + cpu_dev = get_cpu_device(GOLD_LEAD); >> >> Error check cpu_dev here? >> >> > + ret = PTR_ERR_OR_ZERO(opp_temp = >> > + dev_pm_opp_set_supported_hw(cpu_dev,&versions,1)); >> > + if (0 > ret) >> > + goto free_opp; > > The goto here is wrong > >> > + >> > + >> > + ret = PTR_ERR_OR_ZERO(platform_device_register_simple("cpufreq-dt", >> > + -1, NULL, 0)); >> > + >> > + if (0 == ret) >> > + return 0; >> > + >> > +free_opp: >> > + dev_pm_opp_put_supported_hw(opp_temp); >> >> This is not needed because dev_pm_opp_set_supported_hw will free >> memory in case of failure. This call in only needed in case of a >> successful get. > > But this is still required for the case where platform device > registration fails. Agreed. The overall error path needs to be re-written with proper check of return values. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html