> -----Original Message-----
> From: Viresh Kumar <[email protected]>
> Sent: Wednesday, May 16, 2018 17:12
> To: Amit Kucheria <[email protected]>
> Cc: Ilia Lin <[email protected]>; Michael Turquette
> <[email protected]>; [email protected]; Rob Herring
> <[email protected]>; Mark Rutland <[email protected]>; [email protected];
> [email protected]; [email protected]; Andy Gross
> <[email protected]>; David Brown <[email protected]>;
> [email protected]; [email protected]; Rafael J. Wysocki
> <[email protected]>; [email protected];
> [email protected]; LKML <[email protected]>; Linux
> PM list <[email protected]>; [email protected]; linux-
> [email protected]; lakml <[email protected]>;
> Rajendra Nayak <[email protected]>; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v7 12/14] cpufreq: Add Kryo CPU scaling driver
>
> 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
If we are here, then the first dev_pm_opp_set_supported_hw() succeeded. And
should be deallocated before exit with error.
>
> > > +
> > > +
> > > + 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.
>
> --
> viresh
