On 16-06-16, 14:25, Rafael J. Wysocki wrote: > On Thu, Jun 16, 2016 at 8:33 AM, Viresh Kumar <viresh.ku...@linaro.org> wrote: > > + if (opp_table->shared_opp == OPP_TABLE_IS_SHARED) > > + return opp_table; > > + > > + return NULL; > > That still can be > > return opp_table->shared_opp == OPP_TABLE_IS_SHARED ? opp_table : NULL; > > > } > > } > >
> > + if (of_property_read_bool(opp_np, "opp-shared")) > > + opp_table->shared_opp = OPP_TABLE_IS_SHARED; > > + else > > + opp_table->shared_opp = OPP_TABLE_IS_NOT_SHARED; > > And here > > opp_table->shared_opp = of_property_read_bool(opp_np, "opp-shared") ? > OPP_TABLE_IS_SHARED : > OPP_TABLE_IS_NOT_SHARED; Conditional statement for both these cases is getting very long and if/else looks much more readable. And so I would like to stick with that, if you allow. > > +#define OPP_TABLE_IS_NOT_SHARED 0 > > +#define OPP_TABLE_IS_SHARED 1 > > +#define OPP_TABLE_SHARED_UNKNOWN UINT_MAX > > Please change this into an enum type. > Besides, I'd call them OPP_TABLE_ACCESS_SHARED, > OPP_TABLE_ACCESS_EXCLUSIVE, OPP_TABLE_ACCESS_UNKNOWN or similar, but I > don't care that much either. Sure. This can be done. -- viresh
