Hi Georgi, On Fri, Apr 24, 2020 at 06:53:59PM +0300, Georgi Djakov wrote: > From: Saravana Kannan <saravanak@xxxxxxxxxx> > > The opp-hz DT property is not mandatory and we may use another property > as a key in the OPP table. Add helper functions to simplify the reading > and comparing the keys. > > Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx> > Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx> > --- > v7: > * Extracted just the helpers from patch v6, as Viresh advised to split it. > > v6: https://lore.kernel.org/r/20191207002424.201796-3-saravanak@xxxxxxxxxx > > drivers/opp/core.c | 15 +++++++++++++-- > drivers/opp/of.c | 42 ++++++++++++++++++++++++++---------------- > drivers/opp/opp.h | 1 + > 3 files changed, 40 insertions(+), 18 deletions(-) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index ba43e6a3dc0a..c9c1bbe6ae27 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -1272,11 +1272,21 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp, > return true; > } > > +int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2) > +{ > + if (opp1->rate != opp2->rate) > + return opp1->rate < opp2->rate ? -1 : 1; > + if (opp1->level != opp2->level) > + return opp1->level < opp2->level ? -1 : 1; > + return 0; > +} > + > static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp, > struct opp_table *opp_table, > struct list_head **head) > { > struct dev_pm_opp *opp; > + int opp_cmp; > > /* > * Insert new OPP in order of increasing frequency and discard if > @@ -1287,12 +1297,13 @@ static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp, > * loop. > */ > list_for_each_entry(opp, &opp_table->opp_list, node) { > - if (new_opp->rate > opp->rate) { > + opp_cmp = _opp_compare_key(new_opp, opp); > + if (opp_cmp > 0) { > *head = &opp->node; > continue; > } > > - if (new_opp->rate < opp->rate) > + if (opp_cmp < 0) > return 0; > > /* Duplicate OPPs */ > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > index 9cd8f0adacae..e33169c7e045 100644 > --- a/drivers/opp/of.c > +++ b/drivers/opp/of.c > @@ -521,6 +521,28 @@ void dev_pm_opp_of_remove_table(struct device *dev) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); > > +static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np, > + bool *rate_not_available) > +{ > + u64 rate; > + int ret; > + > + ret = of_property_read_u64(np, "opp-hz", &rate); > + if (!ret) { > + /* > + * Rate is defined as an unsigned long in clk API, and so > + * casting explicitly to its type. Must be fixed once rate is 64 > + * bit guaranteed in clk API. > + */ > + new_opp->rate = (unsigned long)rate; > + } nit: curly braces are not needed > + *rate_not_available = !!ret; > + > + of_property_read_u32(np, "opp-level", &new_opp->level); > + > + return ret; > +} > + > /** > * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) > * @opp_table: OPP table > @@ -558,26 +580,14 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, > if (!new_opp) > return ERR_PTR(-ENOMEM); > > - ret = of_property_read_u64(np, "opp-hz", &rate); > + ret = _read_opp_key(new_opp, np, &rate_not_available); > if (ret < 0) { > - /* "opp-hz" is optional for devices like power domains. */ > - if (!opp_table->is_genpd) { > - dev_err(dev, "%s: opp-hz not found\n", __func__); > - goto free_opp; > - } > + if (!opp_table->is_genpd) > + dev_err(dev, "%s: opp key field not found\n", __func__); > > - rate_not_available = true; > - } else { > - /* > - * Rate is defined as an unsigned long in clk API, and so > - * casting explicitly to its type. Must be fixed once rate is 64 > - * bit guaranteed in clk API. > - */ > - new_opp->rate = (unsigned long)rate; > + goto free_opp; > } > > - of_property_read_u32(np, "opp-level", &new_opp->level); > - > /* Check if the OPP supports hardware's hierarchy of versions or not */ > if (!_opp_is_supported(dev, opp_table, np)) { > dev_dbg(dev, "OPP not supported by hardware: %llu\n", rate); > diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h > index d14e27102730..bcadb1e328a4 100644 > --- a/drivers/opp/opp.h > +++ b/drivers/opp/opp.h > @@ -211,6 +211,7 @@ struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_ > void _dev_pm_opp_find_and_remove_table(struct device *dev); > struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table); > void _opp_free(struct dev_pm_opp *opp); > +int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2); > int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available); > int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic); > void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, int last_cpu); Reviewed-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
